Re: [PATCHES] [HACKERS] BEGIN inside transaction should be an error

2006-05-26 Thread Tom Lane
"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

2006-05-26 Thread Alvaro Herrera
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

2006-05-26 Thread Gurjeet Singh

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

2006-05-26 Thread Gurjeet Singh

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

2006-05-26 Thread Andrew Dunstan


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

2006-05-26 Thread Alvaro Herrera
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

2006-05-26 Thread Gurjeet Singh

   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