Re: BUG #17504: psql --single-transaction -vON_ERROR_STOP=1 still commits after client-side error

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, christoph(dot)berg(at)credativ(dot)de, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #17504: psql --single-transaction -vON_ERROR_STOP=1 still commits after client-side error
Date: 2022-06-13 06:45:05
Message-ID: Yqbc8bAdwnP02na4@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Fri, Jun 10, 2022 at 12:12:11PM -0400, Tom Lane wrote:
> Alternatively, one could argue that once we've decided to issue
> ROLLBACK, there's not really any reason to continue performing
> additional -c/-f actions, so that a sufficient fix would be to
> get rid of the loop's ON_ERROR_STOP conditionality:
>
> - if (successResult != EXIT_SUCCESS && pset.on_error_stop)
> + if (successResult != EXIT_SUCCESS)
> break;

This suggestion does not look right to me with --single-transaction.
If not using ON_ERROR_STOP, I think that we should still loop through
all the switches given by the caller and force a COMMIT all the time
because the intention is that we don't care about failures while
processing. This gives me the attached as a result for HEAD, where we
would issue a ROLLBACK only when using ON_ERROR_STOP to stop
immediately if there is a backend-side error or a client-side error.
I have expanded the tests where not using ON_ERROR_STOP, with errors
triggered at the end of the set of switches.

Now, do we really want to introduce this new behavior on HEAD? We are
post-beta so this does not me make me really comfortable if both
Robert and you don't like the change, even if the behavior of
--single-transaction/ON_ERROR_STOP on client-side failure is weird to
me and others from upthread. If you'd prefer see the original logic
in place, I don't mind putting the code in its previous shape.
However, I would like to keep all those TAP tests because we have zero
coverage in this area, and we have the base file to be able to do so
now.
--
Michael

Attachment Content-Type Size
psql-on-error.patch text/x-diff 5.3 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Kyotaro Horiguchi 2022-06-13 07:07:01 Re: BUG #17504: psql --single-transaction -vON_ERROR_STOP=1 still commits after client-side error
Previous Message Noah Misch 2022-06-13 03:45:51 Re: Extension pg_trgm, permissions and pg_dump order