Thread: Re: [HACKERS] BEGIN inside transaction should be an error
I have made the changes to implement this TODO item. I wish to know the standard procedure (command) to generate a patch; and from which level in the source directory should I execute it? On 5/18/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > > Added to TODO: > > * Add a GUC to control whether BEGIN inside a transcation should abort > the transaction. > >
Gurjeet Singh wrote: > I have made the changes to implement this TODO item. I wish to > know the standard procedure (command) to generate a patch; and from > which level in the source directory should I execute it? The toplevel directory. The command is cvs -q diff -cp If you created new files to implement a patch, include them separately, indicating in the patch description in which directory they are meant to reside. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Please read the developers FAQ: http://www.postgresql.org/docs/faqs.FAQ_DEV.html For the most part, patches are probably best generated relative to the root directory of your CVS checkout. cheers andrew Gurjeet Singh wrote: > I have made the changes to implement this TODO item. I wish to > know the standard procedure (command) to generate a patch; and from > which level in the source directory should I execute it? > > On 5/18/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: >> >> Added to TODO: >> >> * Add a GUC to control whether BEGIN inside a transcation >> should abort >> the transaction. >>
On 5/26/06, Alvaro Herrera <alvherre@commandprompt.com> wrote:
> Gurjeet Singh wrote:
> > I wish to
> > know the standard procedure (command) to generate a patch; and from
> > which level in the source directory should I execute it?
>
> The toplevel directory. The command is
>
> cvs -q diff -cp
>
> If you created new files to implement a patch, include them separately,
> indicating in the patch description in which directory they are meant to
> reside.
Thanks.
Here's the patch:
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.220
diff -c -p -r1.220 xact.c
*** src/backend/access/transam/xact.c 25 Apr 2006 00:25:17 -0000 1.220
--- src/backend/access/transam/xact.c 21 May 2006 15:40:00 -0000
*************** bool XactReadOnly;
*** 59,64 ****
--- 59,65 ----
int CommitDelay = 0; /* precommit delay in microseconds */
int CommitSiblings = 5; /* # concurrent xacts needed to sleep */
+ bool BeginInXactIsError = true;
/*
* transaction states - transaction state from server perspective
*************** BeginTransactionBlock(void)
*** 2725,2731 ****
case TBLOCK_SUBINPROGRESS:
case TBLOCK_ABORT:
case TBLOCK_SUBABORT:
! ereport(WARNING,
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg("there is already a transaction in progress")));
break;
--- 2726,2732 ----
case TBLOCK_SUBINPROGRESS:
case TBLOCK_ABORT:
case TBLOCK_SUBABORT:
! ereport(ERROR,
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg("there is already a transaction in progress")));
break;
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.318
diff -c -p -r1.318 guc.c
*** src/backend/utils/misc/guc.c 2 May 2006 11:28:55 -0000 1.318
--- src/backend/utils/misc/guc.c 21 May 2006 15:14:22 -0000
***************
*** 65,70 ****
--- 65,71 ----
#include "utils/pg_locale.h"
#include "pgstat.h"
#include "access/gin.h"
+ #include "access/xact.h"
#ifndef PG_KRB_SRVTAB
#define PG_KRB_SRVTAB ""
*************** static struct config_bool ConfigureNames
*** 1005,1010 ****
--- 1006,1021 ----
false, NULL, NULL
},
+ {
+ {"begin_inside_transaction_is_error", PGC_USERSET, COMPAT_OPTIONS,
+ gettext_noop("A BEGIN statement inside a transaction raises ERROR."),
+ gettext_noop("It is provided to catch buggy applications. "
+ "Disable it to let the buggy application run.")
+ },
+ &BeginInXactIsError,
+ true, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL
Index: src/include/access/xact.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/access/xact.h,v
retrieving revision 1.82
diff -c -p -r1.82 xact.h
*** src/include/access/xact.h 25 Apr 2006 00:25:19 -0000 1.82
--- src/include/access/xact.h 21 May 2006 15:13:10 -0000
*************** extern int XactIsoLevel;
*** 41,46 ****
--- 41,49 ----
extern bool DefaultXactReadOnly;
extern bool XactReadOnly;
+ /* A BEGIN statement inside a transaction raises ERROR */
+ extern bool BeginInXactIsError;
+
/*
* start- and end-of-transaction callbacks for dynamically loaded modules
*/
On 5/26/06, Andrew Dunstan <andrew@dunslane.net> wrote: > > Please read the developers FAQ: > http://www.postgresql.org/docs/faqs.FAQ_DEV.html > > For the most part, patches are probably best generated relative to the > root directory of your CVS checkout. > > cheers > > andrew > > Gurjeet Singh wrote: > > I wish to > > know the standard procedure (command) to generate a patch; and from > > which level in the source directory should I execute it? > > > > On 5/18/06, Bruce Momjian <pgman@candle.pha.pa.us> wrote: > >> > >> Added to TODO: > >> > >> * Add a GUC to control whether BEGIN inside a transcation > >> should abort > >> the transaction. Thanks Andrew. Reposting the patch since my version on guc.c wasn't at the HEAD. Index: src/backend/access/transam/xact.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xact.c,v retrieving revision 1.220 diff -c -p -r1.220 xact.c *** src/backend/access/transam/xact.c 25 Apr 2006 00:25:17 -0000 1.220 --- src/backend/access/transam/xact.c 21 May 2006 15:40:00 -0000 *************** bool XactReadOnly; *** 59,64 **** --- 59,65 ---- int CommitDelay = 0; /* precommit delay in microseconds */ int CommitSiblings = 5; /* # concurrent xacts needed to sleep */ + bool BeginInXactIsError = true; /* * transaction states - transaction state from server perspective *************** BeginTransactionBlock(void) *** 2725,2731 **** case TBLOCK_SUBINPROGRESS: case TBLOCK_ABORT: case TBLOCK_SUBABORT: ! ereport(WARNING, (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION), errmsg("there is already a transaction in progress"))); break; --- 2726,2732 ---- case TBLOCK_SUBINPROGRESS: case TBLOCK_ABORT: case TBLOCK_SUBABORT: ! ereport(ERROR, (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION), errmsg("there is already a transaction in progress"))); break; Index: src/backend/utils/misc/guc.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v retrieving revision 1.320 diff -c -p -r1.320 guc.c *** src/backend/utils/misc/guc.c 21 May 2006 20:10:42 -0000 1.320 --- src/backend/utils/misc/guc.c 26 May 2006 16:10:40 -0000 *************** *** 66,71 **** --- 66,72 ---- #include "utils/pg_locale.h" #include "pgstat.h" #include "access/gin.h" + #include "access/xact.h" #ifndef PG_KRB_SRVTAB #define PG_KRB_SRVTAB "" *************** static struct config_bool ConfigureNames *** 1008,1013 **** --- 1009,1024 ---- false, NULL, NULL }, + { + {"begin_inside_transaction_is_error", PGC_USERSET, COMPAT_OPTIONS, + gettext_noop("A BEGIN statement inside a transaction raises ERROR."), + gettext_noop("It is provided to catch buggy applications. " + "Disable it to let the buggy application run.") + }, + &BeginInXactIsError, + true, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL Index: src/include/access/xact.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/include/access/xact.h,v retrieving revision 1.82 diff -c -p -r1.82 xact.h *** src/include/access/xact.h 25 Apr 2006 00:25:19 -0000 1.82 --- src/include/access/xact.h 21 May 2006 15:13:10 -0000 *************** extern int XactIsoLevel; *** 41,46 **** --- 41,49 ---- extern bool DefaultXactReadOnly; extern bool XactReadOnly; + /* A BEGIN statement inside a transaction raises ERROR */ + extern bool BeginInXactIsError; + /* * start- and end-of-transaction callbacks for dynamically loaded modules */
Gurjeet Singh wrote:
> *************** BeginTransactionBlock(void)
> *** 2725,2731 ****
> case TBLOCK_SUBINPROGRESS:
> case TBLOCK_ABORT:
> case TBLOCK_SUBABORT:
> ! ereport(WARNING,
> (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
> errmsg("there is already a
> transaction in progress")));
> break;
> --- 2726,2732 ----
> case TBLOCK_SUBINPROGRESS:
> case TBLOCK_ABORT:
> case TBLOCK_SUBABORT:
> ! ereport(ERROR,
> (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
> errmsg("there is already a
> transaction in progress")));
> break;
This should depend on the GUC variable for the patch to work at all ...
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
"Gurjeet Singh" <singh.gurjeet@gmail.com> writes:
> Here's the patch:
Wrong default (there was no consensus for changing the default behavior,
and you need to tone down the description strings). A less verbose
GUC variable name would be a good idea, too. Something like
"error_double_begin" would be more in keeping with most of our other names.
Doesn't actually *honor* the GUC variable, just changes the behavior
outright. This betrays a certain lack of testing.
Also, lacks documentation. Please grep the tree for all references to
some existing GUC variable(s) to see what you missed.
regards, tom lane