Date: Thu, 30 Mar 2023 10:07:28 +0200
From: Denis Laxalde <denis.laxalde@dalibo.com>
To: Jelte Fennema <postgres@jeltef.nl>
Cc: Greg Stark <stark@mit.edu>, Tom Lane <tgl@sss.pgh.pa.us>,
	"Gregory Stark (as CFM)" <stark.cfm@gmail.com>,
	Jelte Fennema <Jelte.Fennema@microsoft.com>,
	Daniel Gustafsson <daniel@yesql.se>,
	Peter Eisentraut <peter@eisentraut.org>,
	Jacob Champion <jchampion@timescale.com>,
	Andres Freund <andres@anarazel.de>,
	Justin Pryzby <pryzby@telsasoft.com>,
	Robert Haas <robertmhaas@gmail.com>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Subject: Re: [EXTERNAL] Re: Add non-blocking version of PQcancel
Message-ID: <ZCVDQDMOpvr0aAzm@dalibo.com>
References: <CAGECzQR57o1wdo9fG7grmmabrt1jotHR-T=nQZGXiZRHpvgrGg@mail.gmail.com>
 <CAM-w4HOf4+em6TivqZXtbVvpahHRYExxosK5eAyje5C+dSxWSg@mail.gmail.com>
 <2755358.1678816706@sss.pgh.pa.us>
 <CAM-w4HPyCqVuf=fR-VBB1xcCtUF0+cefpVOv29O-JYbAG4dh2w@mail.gmail.com>
 <CAGECzQR7kRY3WKKwx0FdmAA3P2vD+SGALOcXV-dcqMwM4LHV8A@mail.gmail.com>
 <CAGECzQTaGtAWmM98q9uup2pjODR8WSOsF_UV3YG7THoqatJApA@mail.gmail.com>
 <20230328145318.upvrlslcbjyh47ce@dalibo.com>
 <CAGECzQQUw3UFP8ivMYg0tt3-vDscSZMRuwpABjkUO2m9=JUSTA@mail.gmail.com>
 <20230329084314.higpoqp4aqpkg5iq@dalibo.com>
 <CAGECzQRsOobw4BYbbk08rhxG9FJRRja7=Vn9+EpYA=UG_Bd6NQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="uuvnty6HZk0W25lo"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAGECzQRsOobw4BYbbk08rhxG9FJRRja7=Vn9+EpYA=UG_Bd6NQ@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ZCVDQDMOpvr0aAzm%40dalibo.com>
Precedence: bulk


--uuvnty6HZk0W25lo
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Jelte Fennema a écrit :
> On Wed, 29 Mar 2023 at 10:43, Denis Laxalde <denis.laxalde@dalibo.com> wrote:
> > More importantly, not having PQcancelSend() creating the PGcancelConn
> > makes reuse of that value, passing through PQcancelReset(), more
> > intuitive. E.g., in the tests:
> 
> You convinced me. Attached is an updated patch where PQcancelSend
> takes the PGcancelConn and returns 1 or 0.

Patch 5 is missing respective changes; please find attached a fixup
patch for these.

--uuvnty6HZk0W25lo
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="0001-fixup-Start-using-new-libpq-cancel-APIs.patch"

From c9e59fb3e30db1bfab75be9fdd4afbc227a5270e Mon Sep 17 00:00:00 2001
From: Denis Laxalde <denis.laxalde@dalibo.com>
Date: Thu, 30 Mar 2023 09:19:18 +0200
Subject: [PATCH] fixup! Start using new libpq cancel APIs

---
 contrib/dblink/dblink.c              | 4 ++--
 src/fe_utils/connect_utils.c         | 4 +++-
 src/test/isolation/isolationtester.c | 4 ++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index e139f66e11..073795f088 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -1332,11 +1332,11 @@ dblink_cancel_query(PG_FUNCTION_ARGS)
 
 	dblink_init();
 	conn = dblink_get_named_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)));
-	cancelConn = PQcancelSend(conn);
+	cancelConn = PQcancelConn(conn);
 
 	PG_TRY();
 	{
-		if (PQcancelStatus(cancelConn) == CONNECTION_BAD)
+		if (!PQcancelSend(cancelConn))
 		{
 			msg = pchomp(PQcancelErrorMessage(cancelConn));
 		}
diff --git a/src/fe_utils/connect_utils.c b/src/fe_utils/connect_utils.c
index b32448c010..1cfd717217 100644
--- a/src/fe_utils/connect_utils.c
+++ b/src/fe_utils/connect_utils.c
@@ -161,7 +161,9 @@ disconnectDatabase(PGconn *conn)
 
 	if (PQtransactionStatus(conn) == PQTRANS_ACTIVE)
 	{
-		PQcancelFinish(PQcancelSend(conn));
+		PGcancelConn *cancelConn = PQcancelConn(conn);
+		PQcancelSend(cancelConn);
+		PQcancelFinish(cancelConn);
 	}
 
 	PQfinish(conn);
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 3781f7982b..de31a87571 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -946,9 +946,9 @@ try_complete_step(TestSpec *testspec, PermutationStep *pstep, int flags)
 			 */
 			if (td > max_step_wait && !canceled)
 			{
-				PGcancelConn *cancel_conn = PQcancelSend(conn);
+				PGcancelConn *cancel_conn = PQcancelConn(conn);
 
-				if (PQcancelStatus(cancel_conn) == CONNECTION_OK)
+				if (PQcancelSend(cancel_conn))
 				{
 					/*
 					 * print to stdout not stderr, as this should appear in
-- 
2.30.2


--uuvnty6HZk0W25lo--