Re: [GENERAL] Rollback on include error in psql

On 12/29/2014 09:38 AM, David Johnston wrote:
> Copying -bugs to gain broader attention and opinions.
>
> On Mon, Dec 29, 2014 at 10:06 AM, Adrian Klaver
> <adrian.klaver@aklaver.com <mailto:adrian.klaver@aklaver.com>>wrote:
>
>     On 12/29/2014 08:49 AM, David Johnston wrote:
>
>         On Mon, Dec 29, 2014 at 9:39 AM, Adrian Klaver
>         <adrian.klaver@aklaver.com <mailto:adrian.klaver@aklaver.com>
>         <mailto:adrian.klaver@aklaver.__com
>         <mailto:adrian.klaver@aklaver.com>>>wrote:
>
>
>              On 12/29/2014 07:59 AM, David Johnston wrote:
>
>
>                  Anyway, the third undocumented bug is that
>         --single-transactions
>                  gets to
>                  send its COMMIT even if ON_ERROR_STOP​
>                  ​takes hold before the end of the script.  I imagined
>         it such
>                  that only
>                  if every statement in the "-f <script>" was called
>         would the
>                  COMMIT be
>                  issued - thus the error_stop would supercede and leave
>         the session
>                  uncommitted and by default rolledback.
>
>
>              Not seeing the bug. --single-transaction wraps the entire
>         script in
>              BEGIN/COMMIT, ON_ERROR_STOP stops 'processing' the command,
>         nothing
>              in there about stopping transaction or rollback. So the
>         failed \i
>              stops the script from processing anything after that and
>         the session
>              goes directly to the COMMIT. If you want to deal with
>         transactions
>              there is ON_ERROR_ROLLBACK. Though I did find something
>         interesting
>              about that, which will subject of another post.
>
>
>         ​Then --single-transaction has nothing to do with the script file
>         at-all.  It should be documented as issuing a BEGIN at session
>         connect
>         and a COMMIT just before session disconnect - regardless of
>         whether the
>         named script executes to completion, which can happen if it is
>         combined
>         with ON_ERROR_STOP.
>
>
>     Seems to me when you do:
>
>     psql  --single-transaction -f some_script
>
>     the script is the session.
>
>     ON_ERROR_STOP
>     " ..psql will exit, returning error code 3 to distinguish this case
>     from fatal error conditions, which are reported using error code 1"
>
>     So psql does not see this a fatal error.
>
>     This is one of those glass half full/empty situations, where it is
>     down to the eye of the beholder. I would also say this a perfect
>     example of why tests are written, to see what actually happens
>     versus what you think happens.
>
>
> ​If a user of our product needs to run a test to determine behavior then
> our documentation is flawed - which is the point I am making.

Still not seeing the flaw in the documentation.

>
> ​psql does not see any error due to meta-commands or SQL as fatal -
> which is why the ON_ERROR_STOP option exists.

And ON_ERROR_STOP does not change that. All it does is toggle whether
psql continues on after an error or stops processing commands.

>
> I believe that if ON_ERROR_STOP causes an abort that the COMMIT from
> --single-transaction should not run.  That is a behavior change.  But
> not documenting the known and deterministic interaction between the two
> options is a bug.

I am not seeing anything in the below that says an ABORT is issued:

ON_ERROR_STOP

     By default, command processing continues after an error. When this
variable is set, it will instead stop immediately. In interactive mode,
psql will return to the command prompt; otherwise, psql will exit,
returning error code 3 to distinguish this case from fatal error
conditions, which are reported using error code 1. In either case, any
currently running scripts (the top-level script, if any, and any other
scripts which it may have in invoked) will be terminated immediately. If
the top-level command string contained multiple SQL commands, processing
will stop with the current command.

I do see it here though:

ON_ERROR_ROLLBACK

     When on, if a statement in a transaction block generates an error,
the error is ignored and the transaction continues. When interactive,
such errors are only ignored in interactive sessions, and not when
reading script files. When off (the default), a statement in a
transaction block that generates an error aborts the entire transaction.
The on_error_rollback-on mode works by issuing an implicit SAVEPOINT for
you, just before each command that is in a transaction block, and rolls
back to the savepoint on error.


>
> ​Since the undesirable behavior can be easily worked around by simply
> omitting --single-transaction and writing your own BEGIN/COMMIT into the
> script I don't see that there is going to be a high priority or desire
> to change the behavior and introduce a backward incompatibility; fine.
>
> The other two bugs I see are:
>
> 1) it is not documented that "\include" is a valid alias for "\i"
> (simple fix, see meta-command "\c" or "\connect")
>
> 2) the implications of \include being a client-side mechanic and thus,
> invisible to the server, is not well explained.  Specifically that a
> failure to include is the equivalent of simply omitting the statement
> altogether (aside from the psql warning).  i.e., if in an actual
> transaction the server will not issue the standard "error has occurred,
> you must ROLLBACK." message for any subsequent statements in the
> script.  This is probably not to the level of a bug but it is related to
> the ON_ERROR_STOP bug.

I could see improving the wording on this, to let the user know that
includes are on them as Viktor already determined and took action on.

>
> I personally consider the issuance of COMMIT following a determination
> of ON_ERROR_STOP to be a bug as well.  Error handling mechanics should
> take precedence over transaction handling mechanics and if done as such
> the promise of --single-transaction would hold since the failure of
> \include would abort the session and cause an implicit rollback.

>
> David J.​
>


--
Adrian Klaver
adrian.klaver@aklaver.com