Re: Rollback on include error in psql

On Mon, Dec 29, 2014 at 3:37 PM, Adrian Klaver <adrian(dot)klaver(at)aklaver(dot)com>
wrote:

> 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 am not seeing what point you are trying to make here.​ psql exits - my
contention is that it should do so before issuing "COMMIT;" if
--single-transaction was specified. I really don't care what made psql
exit - a fatal error or a non-fatal one while running under ON_ERROR_STOP.

I can find out the root cause by checking for either a 3 or a 1 but what am
I supposed to do with that information? More specifically, what should I
do if I see a 3 that I wouldn't do if I see a 1; and vice-versa. As a user
I really don't care I just want to know that any changes my script may have
performed prior to the error have been rolled back if psql exits with a
non-zero status.

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

​The transaction doesn't - but psql allows me to do non-SQL stuff along
side of SQL stuff and I want the entire thing to fail if either the SQL or
the non-SQL stuff has a problem. It is incumbent upon psql to make the
boundary between the two as invisible as possible and right now it does not
do as good a job as it could.

From the standpoint of psql \include should be just as much a part of the
transaction as SELECT * FROM tbl - at least when operating in file/script
mode. My issue is with psql - how it manages the underlying
session/transaction to make that works is its problem and should be an
implementation detail I do not have to worry about.

Note: This all likely extends to "\!" as well but I haven't gone and
explored that dynamic.

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

​Why? Most script languages will report an error to the user if a
specified file is missing and provide them a means to respond to that
error. psql lacks formal error handling capabilities (e.g., try/catch​)
but it does offer ON_ERROR_STOP and users should be able to rely on that to
behave in a sane manner - i.e., STOPping - without explicitly committing -
since something went wrong.

​David J.​