Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 2 Jun 2022 14:39:41 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: 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
Message-ID: <YphNHbOkg22H7d52@paquier.xyz>
References: <17504-76b68018e130415e@postgresql.org>
 <YpTgRGknCLBopP8C@msg.df7cb.de>
 <20220531.113927.640026319524742851.horikyota.ntt@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="0lZZAiWmiIOgudmD"
Content-Disposition: inline
In-Reply-To: <20220531.113927.640026319524742851.horikyota.ntt@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/YphNHbOkg22H7d52%40paquier.xyz>
Precedence: bulk


--0lZZAiWmiIOgudmD
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, May 31, 2022 at 11:39:27AM +0900, Kyotaro Horiguchi wrote:
> The code looks like just a thinko that "COMMIT" works fine even if the
> given commands have ended in failure. But actually it doesn't for
> client-side failure.
>=20
> diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
> index ddff903915..2261f78f81 100644
> --- a/src/bin/psql/startup.c
> +++ b/src/bin/psql/startup.c
> @@ -426,7 +426,9 @@ main(int argc, char *argv[])
> =20
>                 if (options.single_txn)
>                 {
> -                       if ((res =3D PSQLexec("COMMIT")) =3D=3D NULL)
> +                       res =3D PSQLexec(successResult =3D=3D EXIT_SUCCES=
S ?
> +                                                  "COMMIT" : "ROLLBACK");
> +                       if (res =3D=3D NULL)
>                         {
>                                 if (pset.on_error_stop)
>                                 {

Yeah, it seems a bit strange to commit the changes if an error happens
on the client side, and the docs are a bit blurry about that because
it has never been considered, I guess.  This would not happen with a
failure in the backend as COMMIT would just map to a ROLLBACK
automatically.

The change that you are sending would enforce this policy as Christoph
would like.  Some tests would be nice to check such behaviors, say in
001_basic.pl, but we also need to be careful when sending down queries
with psql expected to fail because of SIGPIPE (c757a3d, 6d41dd0).  The
docs need a refresh, they mention now that COMMIT is sent after the
last command but that would not be the case anymore with this patch if
there is a client-side error.
--
Michael

--0lZZAiWmiIOgudmD
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmKYTR0ACgkQnvQgOdby
QH2qwRAAjEtWSVhQb+5hZgFbYJjkgPHFVUsQ1Js4zOsxQPjs0RFbJ6AUMnaDTlqi
kX/Z9CpJyHss1hctxz+7Y5Q1hCAyjNW5wNeHiq8ets7SPp6LHccveU2pa+0Drv3j
KSSCmsgGGwJBonpoZ7TShoGtgUvK8bXTePM/wC0GMga5fviJgEO12jL2M0AV7qMk
NXnVOdpBn3YWlF+/ganZfCKYGDUh9IxJxUCqRZXaQ2Vj87O+h6zM8IOdPCfzQgpr
5cJtbp4shIU8cnVaLluq4GOxIeilsdOBf3dIMR6/uRsRArB+/X5ix/Qz0l3wTzqu
+S50HEmHVqGrl15hZRciWJpVPXqzp8ao0eZPtSlW+DETXQJs3VU9RA/qYKqpM4H+
80GOC+hX/VnIPPs68PhDUNUbDD8lo4BR6sSsev4GE3skQ/msmM5qWuZCvCBCHzs5
sHsUYe09YSPbWw7By8bb0r8MGqeOPO9up5/aYUd2mOxqWXyW5yLXpEmUe7j/i3tw
pk8NzhzlwhWXfUTYfw9HBbmI6ep0TFCHX+3yUIVF5buG9/aG6zNcfIq/jEf7juta
znBDGz3IL8ClxqFJNy5MzXXUmzCLql0ZeHHLL9BfxCbUqYXzsjVfbOwN07VHK3t6
4JVKZ0qKMsXckFsPSBdVS/jumz/wjnwMhHNwLgoLBPySVEKHqEc=
=qXXc
-----END PGP SIGNATURE-----

--0lZZAiWmiIOgudmD--