pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: michael@paquier.xyz
Cc: tgl@sss.pgh.pa.us
Cc: robertmhaas@gmail.com
Cc: christoph.berg@credativ.de
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17504: psql --single-transaction -vON_ERROR_STOP=1 still commits after client-side error
Date: Mon, 13 Jun 2022 16:07:01 +0900 (JST)
Message-ID: <20220613.160701.975499861789120736.horikyota.ntt@gmail.com> (raw)
In-Reply-To: <Yqbc8bAdwnP02na4@paquier.xyz>
References: <2649417.1654875709@sss.pgh.pa.us>
	<2656555.1654877531@sss.pgh.pa.us>
	<Yqbc8bAdwnP02na4@paquier.xyz>

At Mon, 13 Jun 2022 15:45:05 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> 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

Agreed. It is actually a bug that on_error_stop is ignored here.

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


-			res = PSQLexec((successResult == EXIT_SUCCESS) ?
.
+			res = PSQLexec((successResult != EXIT_SUCCESS && pset.on_error_stop) ?

Thanks!

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

+1 for at least add the tests.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center





view thread (22+ messages)  latest in thread

Message-ID: <20220613.160701.975499861789120736.horikyota.ntt@gmail.com>
Permalink:  ../20220613.160701.975499861789120736.horikyota.ntt@gmail.com/
Also on:    postgresql.org/message-id/20220613.160701.975499861789120736.horikyota.ntt@gmail.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: horikyota.ntt@gmail.com, michael@paquier.xyz, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, christoph.berg@credativ.de, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #17504: psql --single-transaction -vON_ERROR_STOP=1 still commits after client-side error
  In-Reply-To: <20220613.160701.975499861789120736.horikyota.ntt@gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox