Re: [PATCHES] Bug in psql (on_error_rollback)

2005-09-20 Thread Bruce Momjian

Good patch. You are right that the original code did not consider that
AUTOCOMMIT would change the transaction state seen by the
ON_ERROR_ROLLBACK check.

Fixed in CVS.  ON_ERROR_ROLLBACK was not in 8.0.X so no need to backpatch.

---

Michael Paesold wrote:
 There is a bug in psql for the new ON_ERROR_ROLLBACK feature. In AUTOCOMMIT 
 off mode it does not work correctly for the first statement.
 
 This is how it works usually:
 
 postgres=# \set AUTOCOMMIT off
 postgres=# \set ON_ERROR_ROLLBACK interactive
 postgres=# SELECT 1;
  ?column?
 --
 1
 (1 row)
 
 postgres=# SELECT a;
 ERROR:  column a does not exist
 postgres=# SELECT 1;
  ?column?
 --
 1
 (1 row)
 
 postgres=# BEGIN;
 WARNING:  there is already a transaction in progress
 BEGIN
 postgres=# ROLLBACK;
 ROLLBACK
 
 
 For the first statement in a transaction after the implicit BEGIN it does 
 not work:
 
 postgres=# ROLLBACK;
 ROLLBACK
 postgres=#
 postgres=# SELECT a;
 ERROR:  column a does not exist
 postgres=# SELECT 1;
 ERROR:  current transaction is aborted, commands ignored until end of 
 transaction block
 
 
 With the attaced patch it works correctly even for the first statement.
 
 postgres=# \set AUTOCOMMIT off
 postgres=# \set ON_ERROR_ROLLBACK interactive
 postgres=# SELECT a;
 ERROR:  column a does not exist
 postgres=# SELECT 1;
  ?column?
 --
 1
 (1 row)
 
 postgres=# BEGIN;
 WARNING:  there is already a transaction in progress
 BEGIN
 postgres=# ABORT;
 ROLLBACK
 
 
 Please check the patch and apply to CVS tip.
 I think it would be good to add regression tests for AUTOCOMMIT and 
 ON_ERROR_ROLLBACK and possibly others. There are currently no regression 
 tests specifically for psql features, but since the regression tests are 
 executed via psql, there would be no problem in creating a set of such 
 tests, right?. I could write some.
 
 Best Regards,
 Michael Paesold 

[ Attachment, skipping... ]

 
 ---(end of broadcast)---
 TIP 2: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073
Index: src/bin/psql/common.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/common.c,v
retrieving revision 1.104
diff -c -c -r1.104 common.c
*** src/bin/psql/common.c   22 Jun 2005 21:14:30 -  1.104
--- src/bin/psql/common.c   20 Sep 2005 21:41:45 -
***
*** 1010,1022 
return false;
}
PQclear(results);
}
!   else if (transaction_status == PQTRANS_INTRANS 
!(rollback_str = GetVariable(pset.vars, 
ON_ERROR_ROLLBACK)) != NULL 
!/* !off and !interactive is 'on' */
!pg_strcasecmp(rollback_str, off) != 0 
!(pset.cur_cmd_interactive ||
! pg_strcasecmp(rollback_str, interactive) != 0))
{
if (on_error_rollback_warning == false  pset.sversion  8)
{
--- 1010,1024 
return false;
}
PQclear(results);
+   transaction_status = PQtransactionStatus(pset.db);
}
! 
!   if (transaction_status == PQTRANS_INTRANS 
!   (rollback_str = GetVariable(pset.vars, ON_ERROR_ROLLBACK)) != 
NULL 
!   /* !off and !interactive is 'on' */
!   pg_strcasecmp(rollback_str, off) != 0 
!   (pset.cur_cmd_interactive ||
!pg_strcasecmp(rollback_str, interactive) != 0))
{
if (on_error_rollback_warning == false  pset.sversion  8)
{

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


[PATCHES] Bug in psql (on_error_rollback)

2005-09-16 Thread Michael Paesold
There is a bug in psql for the new ON_ERROR_ROLLBACK feature. In AUTOCOMMIT 
off mode it does not work correctly for the first statement.


This is how it works usually:

postgres=# \set AUTOCOMMIT off
postgres=# \set ON_ERROR_ROLLBACK interactive
postgres=# SELECT 1;
?column?
--
   1
(1 row)

postgres=# SELECT a;
ERROR:  column a does not exist
postgres=# SELECT 1;
?column?
--
   1
(1 row)

postgres=# BEGIN;
WARNING:  there is already a transaction in progress
BEGIN
postgres=# ROLLBACK;
ROLLBACK


For the first statement in a transaction after the implicit BEGIN it does 
not work:


postgres=# ROLLBACK;
ROLLBACK
postgres=#
postgres=# SELECT a;
ERROR:  column a does not exist
postgres=# SELECT 1;
ERROR:  current transaction is aborted, commands ignored until end of 
transaction block



With the attaced patch it works correctly even for the first statement.

postgres=# \set AUTOCOMMIT off
postgres=# \set ON_ERROR_ROLLBACK interactive
postgres=# SELECT a;
ERROR:  column a does not exist
postgres=# SELECT 1;
?column?
--
   1
(1 row)

postgres=# BEGIN;
WARNING:  there is already a transaction in progress
BEGIN
postgres=# ABORT;
ROLLBACK


Please check the patch and apply to CVS tip.
I think it would be good to add regression tests for AUTOCOMMIT and 
ON_ERROR_ROLLBACK and possibly others. There are currently no regression 
tests specifically for psql features, but since the regression tests are 
executed via psql, there would be no problem in creating a set of such 
tests, right?. I could write some.


Best Regards,
Michael Paesold 


psql.patch
Description: Binary data

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster