Bug in psql (on_error_rollback)

Started by Michael Paesoldalmost 21 years ago2 messagespatches
Jump to latest
#1Michael Paesold
mpaesold@gmx.at

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

Attachments:

psql.patchapplication/octet-stream; name=psql.patchDownload+4-3
#2Bruce Momjian
bruce@momjian.us
In reply to: Michael Paesold (#1)
Re: Bug in psql (on_error_rollback)

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

Attachments:

/bjm/difftext/plainDownload+14-13