Hi Jelte,
I had a look into your patchset (v16), did a quick review and played a
bit with the feature.
Patch 2 is missing the documentation about PQcancelSocket() and contains
a few typos; please find attached a (fixup) patch to correct these.
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -321,16 +328,28 @@ extern PostgresPollingStatusType PQresetPoll(PGconn
*conn);
/* Synchronous (blocking) */
extern void PQreset(PGconn *conn);
+/* issue a cancel request */
+extern PGcancelConn * PQcancelSend(PGconn *conn);
[...]
Maybe I'm missing something, but this function above seems a bit
strange. Namely, I wonder why it returns a PGcancelConn and what's the
point of requiring the user to call PQcancelStatus() to see if something
got wrong. Maybe it could be defined as:
int PQcancelSend(PGcancelConn *cancelConn);
where the return value would be status? And the user would only need to
call PQcancelErrorMessage() in case of error. This would leave only one
single way to create a PGcancelConn value (i.e. PQcancelConn()), which
seems less confusing to me.
Jelte Fennema wrote:
> Especially since I ran into another use case that I would want to use
> this patch for recently: Adding an async cancel function to Python
> it's psycopg3 library. This library exposes both a Connection class
> and an AsyncConnection class (using python its asyncio feature). But
> one downside of the AsyncConnection type is that it doesn't have a
> cancel method.
As part of my testing, I've implemented non-blocking cancellation in
Psycopg, based on v16 on this patchset. Overall this worked fine and
seems useful; if you want to try it:
https://github.com/dlax/psycopg3/tree/pg16/non-blocking-pqcancel
(The only thing I found slightly inconvenient is the need to convey the
connection encoding (from PGconn) when handling error message from the
PGcancelConn.)
Cheers,
Denis
>From a5f9cc680ffa520b05fe34b7cac5df2e60a6d4ad Mon Sep 17 00:00:00 2001
From: Denis Laxalde <denis.laxa...@dalibo.com>
Date: Tue, 28 Mar 2023 16:06:42 +0200
Subject: [PATCH] fixup! Add non-blocking version of PQcancel
---
doc/src/sgml/libpq.sgml | 16 +++++++++++++++-
src/interfaces/libpq/fe-connect.c | 2 +-
src/test/modules/libpq_pipeline/libpq_pipeline.c | 2 +-
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 29f08a4317..aa404c4d15 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -5795,7 +5795,7 @@ PGcancelConn *PQcancelSend(PGconn *conn);
options as the the original <structname>PGconn</structname>. So when the
original connection is encrypted (using TLS or GSS), the connection for
the cancel request is encrypted in the same way. Any connection options
- that only are only used during authentication or after authentication of
+ that are only used during authentication or after authentication of
the client are ignored though, because cancellation requests do not
require authentication and the connection is closed right after the
cancellation request is submitted.
@@ -5912,6 +5912,20 @@ ConnStatusType PQcancelStatus(const PGcancelConn *conn);
</listitem>
</varlistentry>
+ <varlistentry id="libpq-PQcancelSocket">
+ <term><function>PQcancelSocket</function><indexterm><primary>PQcancelSocket</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ A version of <xref linkend="libpq-PQsocket"/> that can be used for
+ cancellation connections.
+<synopsis>
+int PQcancelSocket(PGcancelConn *conn);
+</synopsis>
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-PQcancelPoll">
<term><function>PQcancelPoll</function><indexterm><primary>PQcancelPoll</primary></indexterm></term>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 74e337fddf..16af7303d4 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -808,7 +808,7 @@ PQcancelConn(PGconn *conn)
/*
* Cancel requests should not iterate over all possible hosts. The request
* needs to be sent to the exact host and address that the original
- * connection used. So we we manually create the host and address arrays
+ * connection used. So we manually create the host and address arrays
* with a single element after freeing the host array that we generated
* from the connection options.
*/
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index e8e904892c..6764ab513b 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -89,7 +89,7 @@ pg_fatal_impl(int line, const char *fmt,...)
/*
* Check that the query on the given connection got cancelled.
*
- * This is a function wrapped in a macrco to make the reported line number
+ * This is a function wrapped in a macro to make the reported line number
* in an error match the line number of the invocation.
*/
#define confirm_query_cancelled(conn) confirm_query_cancelled_impl(__LINE__, conn)
--
2.30.2
