Re: Rollback on include error in psql

Copying -bugs to gain broader attention and opinions.

On Mon, Dec 29, 2014 at 10:06 AM, Adrian Klaver <adrian(dot)klaver(at)aklaver(dot)com>
wrote:

> On 12/29/2014 08:49 AM, David Johnston wrote:
>
>> On Mon, Dec 29, 2014 at 9:39 AM, Adrian Klaver
>> <adrian(dot)klaver(at)aklaver(dot)com <mailto:adrian(dot)klaver(at)aklaver(dot)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.

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

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.

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