Re: Rollback on include error in psql

On 12/29/2014 02:28 PM, David Johnston wrote:
> On Mon, Dec 29, 2014 at 3:07 PM, Adrian Klaver
> <adrian.klaver@aklaver.com <mailto:adrian.klaver@aklaver.com>>wrote:
>
>     On 12/29/2014 09:38 AM, David Johnston wrote:
>
>
>              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.
>
>
> ​If it walks and talks like a duck...the fact that ON_ERROR_STOP makes
> psql halt processing means that it now treats them like it does any
> other fatal error.​

But it does not:

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,

<HIGHLIGHT> psql will exit, returning error code 3 to distinguish this
case from fatal error conditions, which are reported using error code
1.<HIGHLIGHT>

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 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:
>
>
> ​I was using term in its non-SQL sense: to stop processing and return
> control to the user.​

So if is non-SQL why should the transaction care about it?

>
>         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 think you have a typo somewhere here 'cause that sentence fragment
> (...includes and on them as) makes no sense to me.​

Should have been clearer. I am saying that it would be good to tell
users that using \i(nclude) puts the burden on them to verify the
included scripts actually can be found.

>
> The overall complaint is that a missing \include file, without
> ON_ERROR_STOP, ​ends up being totally ignored even while in
> non-interactive mode.  I get the benefit to that behavior in interactive
> mode and so being required to use ON_ERROR_STOP in script mode (which is
> the safest practice anyway) isn't that big a deal as long as in that
> mode a failure causes an immediate stop without any other SQL being sent
> to the server and, by extension, the session closing and effecting a
> rollback in the process if in --single-transaction mode just like that
> mode promises.
>
> I'm not sure why --single-transaction even exists TBH.  The script
> should determine its desired transaction modes and not leave the
> decision up to the caller.  If the script relies on all-or-nothing it
> should have explicit BEGIN/COMMIT statements.
>
> That said it does exist so it should play nicely with ON_ERROR_STOP.  It
> currently does not nor is the not-nice interaction documented anywhere.
>
> David J.


--
Adrian Klaver
adrian.klaver@aklaver.com