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