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(dot)klaver(at)aklaver(dot)com <mailto:adrian(dot)klaver(at)aklaver(dot)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(dot)klaver(at)aklaver(dot)com