Re: [PATCHES] [HACKERS] BEGIN inside transaction should be an error
"Gurjeet Singh" <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] BEGIN inside transaction should be an error
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 Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] BEGIN inside transaction should be an error
On 5/26/06, Andrew Dunstan <[EMAIL PROTECTED]> 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 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 - 1.220 --- src/backend/access/transam/xact.c 21 May 2006 15:40:00 - *** boolXactReadOnly; *** 59,64 --- 59,65 intCommitDelay = 0;/* precommit delay in microseconds */ intCommitSiblings = 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.c21 May 2006 20:10:42 - 1.320 --- src/backend/utils/misc/guc.c26 May 2006 16:10:40 - *** *** 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 - 1.82 --- src/include/access/xact.h 21 May 2006 15:13:10 - *** 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 */ ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] BEGIN inside transaction should be an error
On 5/26/06, Alvaro Herrera <[EMAIL PROTECTED]> 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 - 1.220 --- src/backend/access/transam/xact.c 21 May 2006 15:40:00 - *** boolXactReadOnly; *** 59,64 --- 59,65 intCommitDelay = 0;/* precommit delay in microseconds */ intCommitSiblings = 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.c2 May 2006 11:28:55 - 1.318 --- src/backend/utils/misc/guc.c21 May 2006 15:14:22 - *** *** 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 - 1.82 --- src/include/access/xact.h 21 May 2006 15:13:10 - *** 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 */ ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] BEGIN inside transaction should be an error
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 wrote: Added to TODO: * Add a GUC to control whether BEGIN inside a transcation should abort the transaction. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] BEGIN inside transaction should be an error
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [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 wrote: Added to TODO: * Add a GUC to control whether BEGIN inside a transcation should abort the transaction. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq