Re: Continue transactions after errors in psql - Mailing list pgsql-patches

Michael Paesold wrote:
> Greg Sabino Mullane wrote:
>
> > Finally had a chance to sit down at look at this afresh, and I'm
> > pretty sure I've got all the kinks worked out this time. Apologies
> > for not attaching, but my mail system is not working well enough
> > at the moment. So, please try to break this patch:
> >
> > http://www.gtsm.com/pg/psql_error_recovery.diff
>
>
> Some suggestions in random order:
>
> * I think you should use PSQLexec instead of using PQexec directly. PSQLexec
> is used by all \-commands and prints out queries with -E, which is very
> helpful for debugging.
>
>   -E         display queries that internal commands generate

It is now \set ON_ERROR_ROLLBACK, and PQexec seems right for that.
Also, this isn't something like \d where anyone would want to see the
queries, I think.

>
> * You do not check for the server version before activating \reseterror.
>   -> use PQserverVersion() to check for >= 80000

Added.  Patch just posted.

> * Perhaps the name should be \reseterrors (plural)? Just my personal opinion
> though.

Changed, as you see above.

> * If I read the code correctly, you now don't destroy user savepoints
> anymore, but on the other hand, you do not release the psql savepoint after
> a user-defined savepoint is released. In other words, each time a user
> creates a savepoint, one psql savepoint is left on the subxact stack. I
> don't know if this is a real problem, though.

Interesting.   I thought this would fail, but it doesn't:

    test=> \set ON_ERROR_ROLLBACK on
    test=> begin;
    BEGIN
    test=> savepoint user1;
    SAVEPOINT
    test=> lkjasdfsadf;
    ERROR:  syntax error at or near "lkjasdfsadf" at character 1
    LINE 1: lkjasdfsadf;
            ^
    test=> rollback to savepoint user1;
    ROLLBACK

and I think this is the reason:

    test=> BEGIN;
    BEGIN
    test=> SAVEPOINT psql1;
    SAVEPOINT
    test=> SAVEPOINT user1;
    SAVEPOINT
    test=> SAVEPOINT psql1;
    SAVEPOINT
    test=> lkjasd;
    ERROR:  syntax error at or near "lkjasd" at character 1
    LINE 1: lkjasd;
            ^
    test=> ROLLBACK TO psql1;
    ROLLBACK
    test=> ROLLBACK TO user1;
    ROLLBACK

What Greg's code does, effectively, is to move the savepoint down below
the SAVEPOINt/RELEASE/ROLLBACK so it doesn't discard the user command.
Nice trick:

+           /*
+            *   Do nothing if they are messing with savepoints themselves:
+            *   doing otherwise would cause us to remove their savepoint,
+            *   or have us rollback our savepoint they have just removed
+            */
+           if (strcmp(PQcmdStatus(results), "SAVEPOINT") == 0 ||
+               strcmp(PQcmdStatus(results), "RELEASE") == 0 ||
+               strcmp(PQcmdStatus(results), "ROLLBACK") ==0)
+               results = NULL;

> * You have not yet implemented a way to savely put \reseterror in .psqlrc. I
> previously suggested an AUTO setting (additional to ON/OFF) that disables
> \reseterror when reading from a non-tty. So putting \reseterror AUTO in
> .psqlrc would be save.

Good question, or rather, should ON_ERROR_ROLLBACK have an effect when
commands come from a file?  There is no way to test for the error in
psql so it seems you would never want the transaction to continue after
an error.  I am inclined to make ON_ERROR_ROLLBACK work only for
interactive sessions, just like ON_ERROR_STOP works only for
non-interactive sessions.

Comments?

--
  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