On Sat, 30 Dec 2023 at 00:07, Jelte Fennema-Nio <m...@jeltef.nl> wrote:
>
> On Fri, 29 Dec 2023 at 19:32, Jeff Davis <pg...@j-davis.com> wrote:
> > That is my biggest concern right now: what will new clients connecting
> > to old servers do?
>
> This is not that big of a deal. Since it's only an addition of a new
> message type, it's completely backwards compatible with the current
> protocol version. i.e. as long as a client just doesn't send it when
> the server reports an older protocol version everything works fine.
> The protocol already has version negotiation built in and the server
> implements it in a reasonable way. The only problem is that no-one
> bothered to implement the client side of it in libpq, because it
> wasn't necessary yet since 3.x only had a single minor version.
>
> Patch v20230911-0003[5] from thread [3] implements client side
> handling in (imho) a sane way.
Okay I updated this patchset to start with better handling of the
NegotiateProtocolVersion packet. The implementation is quite different
from [5] after all, but the core idea is the same. It also allows the
connection to continue to work in case of a missing protocol
extension, which is necessary for the libpq compression patchset[6].
In case the protocol extension is a requirement, the client can still
choose to disconnect by checking the return value of the newly added
PQunsupportedProtocolExtensions function.
I also fixed the tests of my final patch, but haven't changed the
behaviour of it in any of the suggested ways.
[3]:
https://www.postgresql.org/message-id/flat/AB607155-8FED-4C8C-B702-205B33884CBB%40yandex-team.ru#961c695d190cdccb3975a157b22ce9d8
From ad000807b9f5491ed616b167297f1fdade76f4ee Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Tue, 2 Jan 2024 11:19:54 +0100
Subject: [PATCH v2 3/4] Bump protocol version to 3.1
In preparation of new additions to the protocol in a follow up commit
this bumps the minor protocol version number.
---
src/include/libpq/pqcomm.h | 3 +--
src/interfaces/libpq/fe-connect.c | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 999ab94b9c8..a6a203d4c4f 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -91,11 +91,10 @@ is_unixsock_path(const char *path)
/*
* The earliest and latest frontend/backend protocol version supported.
- * (Only protocol version 3 is currently supported)
*/
#define PG_PROTOCOL_EARLIEST PG_PROTOCOL(3,0)
-#define PG_PROTOCOL_LATEST PG_PROTOCOL(3,0)
+#define PG_PROTOCOL_LATEST PG_PROTOCOL(3,1)
typedef uint32 ProtocolVersion; /* FE/BE protocol version number */
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index aa01443cf03..8a1ca07ab3b 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -2779,7 +2779,7 @@ keep_going: /* We will come back to here until there is
* must persist across individual connection attempts, but we must
* reset them when we start to consider a new server.
*/
- conn->pversion = PG_PROTOCOL(3, 0);
+ conn->pversion = PG_PROTOCOL_LATEST;
conn->send_appname = true;
#ifdef USE_SSL
/* initialize these values based on SSL mode */
--
2.34.1
From 7e32f3d58c2c58d51f1cd5297a5b2c05b578daf1 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Tue, 2 Jan 2024 11:16:19 +0100
Subject: [PATCH v2 1/4] libpq: Handle NegotiateProtocolVersion message more
leniently
Currently libpq would always error when the server returned a
NegotiateProtocolVersion message. This was fine because libpq only
supports a single protocol version and did not support any protocol
extensions. But we now need to change that to be able to add support for
future protocol changes, with a working fallback when connecting to an
older server.
This patch modifies the client side checks to allow a range of supported
protocol versions, instead of only allowing the exact version that was
requested. In addition it now allows connecting when the server does not
support some of the requested protocol extensions.
This patch also adds a new PQunsupportedProtocolExtensions API to libpq,
since a user might want to take some action in case a protocol extension
is not supported.
---
doc/src/sgml/libpq.sgml | 19 ++++++++++++
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-connect.c | 46 ++++++++++++++++++++++++++++-
src/interfaces/libpq/fe-protocol3.c | 46 ++++++++++-------------------
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 2 ++
6 files changed, 83 insertions(+), 32 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index ed88ac001a1..2e9ae41e389 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2576,6 +2576,25 @@ int PQprotocolVersion(const PGconn *conn);
</listitem>
</varlistentry>
+ <varlistentry id="libpq-PQunsupportedProtocolExtensions">
+ <term><function>PQprotocolVersion</function><indexterm><primary>PQprotocolVersion</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ Returns a null-terminated array of protocol extensions that were
+ requested by the client but are not supported by the server.
+<synopsis>
+int PQunsupportedProtocolExtensions(const PGconn *conn);
+</synopsis>
+ Applications might wish to use this function to determine whether certain
+ protocol extensions they intended to use are supported. Even when some
+ extension is not supported the connection can still be used, only the
+ unsupported extensions cannot be used. Returns NULL if the connection is
+ bad.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-PQserverVersion">
<term><function>PQserverVersion</function><indexterm><primary>PQserverVersion</primary></indexterm></term>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 850734ac96c..849617cb9b2 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -191,3 +191,4 @@ PQclosePrepared 188
PQclosePortal 189
PQsendClosePrepared 190
PQsendClosePortal 191
+PQunsupportedProtocolExtensions 192
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index bf83a9b5697..14214cac62d 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -382,6 +382,8 @@ static const PQEnvironmentOption EnvironmentOptions[] =
}
};
+static const char *no_unsupported_protocol_extensions[1] = {NULL};
+
/* The connection URI must start with either of the following designators: */
static const char uri_designator[] = "postgresql://";
static const char short_uri_designator[] = "postgres://";
@@ -3782,9 +3784,25 @@ keep_going: /* We will come back to here until there is
libpq_append_conn_error(conn, "received invalid protocol negotiation message");
goto error_return;
}
+
+ if (conn->pversion < PG_PROTOCOL_EARLIEST)
+ {
+ libpq_append_conn_error(conn, "protocol version not supported by server: client supports down to %u.%u, server supports up to %u.%u",
+ PG_PROTOCOL_MAJOR(PG_PROTOCOL_EARLIEST), PG_PROTOCOL_MINOR(PG_PROTOCOL_EARLIEST),
+ PG_PROTOCOL_MAJOR(conn->pversion), PG_PROTOCOL_MINOR(conn->pversion));
+ goto error_return;
+ }
+
+ /* neither -- server shouldn't have sent it */
+ if (!(conn->pversion < PG_PROTOCOL_LATEST) && !conn->unsupported_pextensions)
+ {
+ libpq_append_conn_error(conn, "invalid %s message", "NegotiateProtocolVersion");
+ goto error_return;
+ }
+
/* OK, we read the message; mark data consumed */
conn->inStart = conn->inCursor;
- goto error_return;
+ goto keep_going;
}
/* It is an authentication request. */
@@ -4411,6 +4429,20 @@ freePGconn(PGconn *conn)
}
free(conn->connhost);
+ if (conn->unsupported_pextensions)
+ {
+ /* clean up unsupported_pextensions entries */
+ int i = 0;
+
+ while (conn->unsupported_pextensions[i])
+ {
+ free(conn->unsupported_pextensions[i]);
+ i++;
+ }
+ free(conn->unsupported_pextensions);
+ }
+
+
free(conn->client_encoding_initial);
free(conn->events);
free(conn->pghost);
@@ -7234,6 +7266,18 @@ PQprotocolVersion(const PGconn *conn)
return PG_PROTOCOL_MAJOR(conn->pversion);
}
+const char **
+PQunsupportedProtocolExtensions(const PGconn *conn)
+{
+ if (!conn)
+ return NULL;
+ if (conn->status == CONNECTION_BAD)
+ return NULL;
+ if (!conn->unsupported_pextensions)
+ return no_unsupported_protocol_extensions;
+ return (const char **) conn->unsupported_pextensions;
+}
+
int
PQserverVersion(const PGconn *conn)
{
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 8c4ec079caa..89ce0d3962f 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1410,49 +1410,33 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
int
pqGetNegotiateProtocolVersion3(PGconn *conn)
{
- int tmp;
- ProtocolVersion their_version;
+ int their_version;
int num;
- PQExpBufferData buf;
- if (pqGetInt(&tmp, 4, conn) != 0)
+ if (pqGetInt(&their_version, 4, conn) != 0)
return EOF;
- their_version = tmp;
if (pqGetInt(&num, 4, conn) != 0)
return EOF;
- initPQExpBuffer(&buf);
- for (int i = 0; i < num; i++)
+ conn->pversion = their_version;
+ if (num)
{
- if (pqGets(&conn->workBuffer, conn))
+ conn->unsupported_pextensions = calloc(num + 1, sizeof(char *));
+ for (int i = 0; i < num; i++)
{
- termPQExpBuffer(&buf);
- return EOF;
+ if (pqGets(&conn->workBuffer, conn))
+ {
+ return EOF;
+ }
+ conn->unsupported_pextensions[i] = strdup(conn->workBuffer.data);
+ if (!conn->unsupported_pextensions[i])
+ {
+ return EOF;
+ }
}
- if (buf.len > 0)
- appendPQExpBufferChar(&buf, ' ');
- appendPQExpBufferStr(&buf, conn->workBuffer.data);
}
- if (their_version < conn->pversion)
- libpq_append_conn_error(conn, "protocol version not supported by server: client uses %u.%u, server supports up to %u.%u",
- PG_PROTOCOL_MAJOR(conn->pversion), PG_PROTOCOL_MINOR(conn->pversion),
- PG_PROTOCOL_MAJOR(their_version), PG_PROTOCOL_MINOR(their_version));
- if (num > 0)
- {
- appendPQExpBuffer(&conn->errorMessage,
- libpq_ngettext("protocol extension not supported by server: %s",
- "protocol extensions not supported by server: %s", num),
- buf.data);
- appendPQExpBufferChar(&conn->errorMessage, '\n');
- }
-
- /* neither -- server shouldn't have sent it */
- if (!(their_version < conn->pversion) && !(num > 0))
- libpq_append_conn_error(conn, "invalid %s message", "NegotiateProtocolVersion");
-
- termPQExpBuffer(&buf);
return 0;
}
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 97762d56f5d..408ba495088 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -347,6 +347,7 @@ extern PGTransactionStatusType PQtransactionStatus(const PGconn *conn);
extern const char *PQparameterStatus(const PGconn *conn,
const char *paramName);
extern int PQprotocolVersion(const PGconn *conn);
+extern const char **PQunsupportedProtocolExtensions(const PGconn *conn);
extern int PQserverVersion(const PGconn *conn);
extern char *PQerrorMessage(const PGconn *conn);
extern int PQsocket(const PGconn *conn);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 7888199b0d9..c379391a6b2 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -461,6 +461,8 @@ struct pg_conn
SockAddr laddr; /* Local address */
SockAddr raddr; /* Remote address */
ProtocolVersion pversion; /* FE/BE protocol version in use */
+ char **unsupported_pextensions; /* Unsupported protocol
+ * extensions, null-terminated */
int sversion; /* server version, e.g. 70401 for 7.4.1 */
bool auth_req_received; /* true if any type of auth req received */
bool password_needed; /* true if server demanded a password */
base-commit: 141752bbb0308a40bdbef3ab0a2ba31162813412
--
2.34.1
From 37b4549f5b89be0da59597c0f1599c627349159e Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Fri, 29 Dec 2023 16:13:38 +0100
Subject: [PATCH v2 4/4] Add support to change GUCs at the protocol level
Currently the only way to set GUCs from a client is by using SET
commands or set them in the StartupMessage. I think it would be very
useful to be able to change settings using a protocol message. For the
following reasons:
1. Protocol messages are much easier to inspect for connection poolers than queries
2. It paves the way for GUCs that can only be set using a protocol message (and not using SET).
3. Being able to change GUCs while in an aborted transaction.
4. Have an easy way to use the result from ParameterStatus, to set a GUC to that value
---
doc/src/sgml/protocol.sgml | 92 ++++++++++
src/backend/tcop/postgres.c | 22 +++
src/include/libpq/protocol.h | 2 +
src/interfaces/libpq/exports.txt | 2 +
src/interfaces/libpq/fe-exec.c | 76 +++++++++
src/interfaces/libpq/fe-protocol3.c | 22 +++
src/interfaces/libpq/fe-trace.c | 21 +++
src/interfaces/libpq/libpq-fe.h | 3 +
src/interfaces/libpq/libpq-int.h | 3 +-
.../modules/libpq_pipeline/libpq_pipeline.c | 157 ++++++++++++++++++
.../libpq_pipeline/t/001_libpq_pipeline.pl | 2 +-
.../libpq_pipeline/traces/parameter_set.trace | 77 +++++++++
12 files changed, 477 insertions(+), 2 deletions(-)
create mode 100644 src/test/modules/libpq_pipeline/traces/parameter_set.trace
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 6c3e8a631d7..f83c0d0fe39 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1368,6 +1368,30 @@ SELCT 1/0;<!-- this typo is intentional -->
</note>
</sect2>
+ <sect2 id="protocol-changing-backend-parameters">
+ <title>Changing backend parameters</title>
+ <para>
+ The ParameterSet message can be used to change the value of a backend
+ paramater. This is almost equivalent to issuing a SET command, but there
+ are a few differences. First, the ParameterSet message is not part of the
+ ongoing transaction. So it will not be rolled back if the transaction is
+ aborted and neither will it be rejected if the transaction is currently in
+ an errored state where it would reject queries. Second, the ParameterSet message is
+ easier for connection poolers to intercept. Finally, in the future, some
+ parameters may be marked as only changable at the protocol level. The
+ response to this message is either ParameterSetComplete or ErrorResponse.
+ </para>
+
+ <note>
+ <para>
+ While ParameterSet is not itself part of any transaction, if it fails it
+ will still abort any currently active transaction. Also if it is sent as
+ part of an already failed pipeline, it will be ignored just like any other
+ messages.
+ </para>
+ </note>
+ </sect2>
+
<sect2 id="protocol-flow-canceling-requests">
<title>Canceling Requests in Progress</title>
@@ -5271,6 +5295,74 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
</listitem>
</varlistentry>
+ <varlistentry id="protocol-message-formats-ParameterSet">
+ <term>ParameterSet (F)</term>
+ <listitem>
+ <variablelist>
+ <varlistentry>
+ <term>Byte1('U')</term>
+ <listitem>
+ <para>
+ Identifies the message as a run-time parameter change.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term>Int32</term>
+ <listitem>
+ <para>
+ Length of message contents in bytes, including self.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term>String</term>
+ <listitem>
+ <para>
+ The name of the run-time parameter to change.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term>String</term>
+ <listitem>
+ <para>
+ The new value of the parameter.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="protocol-message-formats-ParameterSetComplete">
+ <term>ParameterSetComplete (B)</term>
+ <listitem>
+ <variablelist>
+ <varlistentry>
+ <term>Byte1('U')</term>
+ <listitem>
+ <para>
+ Identifies the message as a ParamaterSet-complete indicator.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term>Int32(4)</term>
+ <listitem>
+ <para>
+ Length of message contents in bytes, including self.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="protocol-message-formats-Parse">
<term>Parse (F)</term>
<listitem>
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 7298a187d18..1333bf93447 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -427,6 +427,7 @@ SocketBackend(StringInfo inBuf)
case PqMsg_Describe:
case PqMsg_Execute:
case PqMsg_Flush:
+ case PqMsg_ParameterSet:
maxmsglen = PQ_SMALL_MESSAGE_LIMIT;
doing_extended_query_message = true;
break;
@@ -4851,6 +4852,27 @@ PostgresMain(const char *dbname, const char *username)
send_ready_for_query = true;
break;
+ case PqMsg_ParameterSet:
+ {
+ const char *parameter_name;
+ const char *parameter_value;
+
+ forbidden_in_wal_sender(firstchar);
+
+ parameter_name = pq_getmsgstring(&input_message);
+ parameter_value = pq_getmsgstring(&input_message);
+ pq_getmsgend(&input_message);
+
+ SetConfigOption(
+ parameter_name,
+ parameter_value,
+ PGC_USERSET,
+ PGC_S_CLIENT);
+ if (whereToSendOutput == DestRemote)
+ pq_putemptymessage(PqMsg_ParameterSetComplete);
+ }
+ break;
+
/*
* 'X' means that the frontend is closing down the socket. EOF
* means unexpected loss of frontend connection. Either way,
diff --git a/src/include/libpq/protocol.h b/src/include/libpq/protocol.h
index cc46f4b586a..bdbd1356da8 100644
--- a/src/include/libpq/protocol.h
+++ b/src/include/libpq/protocol.h
@@ -25,6 +25,7 @@
#define PqMsg_Parse 'P'
#define PqMsg_Query 'Q'
#define PqMsg_Sync 'S'
+#define PqMsg_ParameterSet 'U'
#define PqMsg_Terminate 'X'
#define PqMsg_CopyFail 'f'
#define PqMsg_GSSResponse 'p'
@@ -52,6 +53,7 @@
#define PqMsg_RowDescription 'T'
#define PqMsg_FunctionCallResponse 'V'
#define PqMsg_CopyBothResponse 'W'
+#define PqMsg_ParameterSetComplete 'U'
#define PqMsg_ReadyForQuery 'Z'
#define PqMsg_NoData 'n'
#define PqMsg_PortalSuspended 's'
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 849617cb9b2..9df8281965a 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -192,3 +192,5 @@ PQclosePortal 189
PQsendClosePrepared 190
PQsendClosePortal 191
PQunsupportedProtocolExtensions 192
+PQparameterSet 193
+PQsendParameterSet 194
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index b9511df2c26..a4aa223c345 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -3346,6 +3346,82 @@ PQsendFlushRequest(PGconn *conn)
return 1;
}
+/*
+ * PQparameterSet
+ * Send a request for the server to change a run-time parameter setting.
+ *
+ * If the query was not even sent, return NULL; conn->errorMessage is set to
+ * a relevant message.
+ * If the query was sent, a new PGresult is returned (which could indicate
+ * either success or failure). On success, the PGresult contains status
+ * PGRES_COMMAND_OK. The user is responsible for freeing the PGresult via
+ * PQclear() when done with it.
+ */
+PGresult *
+PQparameterSet(PGconn *conn, const char *parameter, const char *value)
+{
+ if (!PQexecStart(conn))
+ return NULL;
+ if (!PQsendParameterSet(conn, parameter, value))
+ return NULL;
+ return PQexecFinish(conn);
+}
+
+/*
+ * PQsendParameterSet
+ * Send a request for the server to change a run-time parameter setting.
+ *
+ * Returns 1 on success and 0 on failure.
+ */
+int
+PQsendParameterSet(PGconn *conn, const char *parameter, const char *value)
+{
+ PGcmdQueueEntry *entry = NULL;
+
+ if (!PQsendQueryStart(conn, true))
+ return 0;
+
+ entry = pqAllocCmdQueueEntry(conn);
+ if (entry == NULL)
+ return 0; /* error msg already set */
+
+ /* construct the Close message */
+ if (pqPutMsgStart(PqMsg_ParameterSet, conn) < 0 ||
+ pqPuts(parameter, conn) < 0 ||
+ pqPuts(value, conn) < 0 ||
+ pqPutMsgEnd(conn) < 0)
+ goto sendFailed;
+
+ /* construct the Sync message */
+ if (conn->pipelineStatus == PQ_PIPELINE_OFF)
+ {
+ if (pqPutMsgStart(PqMsg_Sync, conn) < 0 ||
+ pqPutMsgEnd(conn) < 0)
+ goto sendFailed;
+ }
+
+ entry->queryclass = PGQUERY_PARAMETER_SET;
+
+ /*
+ * Give the data a push (in pipeline mode, only if we're past the size
+ * threshold). In nonblock mode, don't complain if we're unable to send
+ * it all; PQgetResult() will do any additional flushing needed.
+ */
+ if (pqPipelineFlush(conn) < 0)
+ goto sendFailed;
+
+ /* OK, it's launched! */
+ pqAppendCmdQueueEntry(conn, entry);
+
+ return 1;
+
+sendFailed:
+ pqRecycleCmdQueueEntry(conn, entry);
+ /* error message should be set up already */
+ return 0;
+}
+
+
/* ====== accessor funcs for PGresult ======== */
ExecStatusType
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 89ce0d3962f..76c52d77695 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -297,6 +297,28 @@ pqParseInput3(PGconn *conn)
conn->asyncStatus = PGASYNC_READY;
}
break;
+ case PqMsg_ParameterSetComplete:
+
+ /*
+ * If we're doing PQsendParameterSet, we're done; else
+ * ignore
+ */
+ if (conn->cmd_queue_head &&
+ conn->cmd_queue_head->queryclass == PGQUERY_PARAMETER_SET)
+ {
+ if (!pgHavePendingResult(conn))
+ {
+ conn->result = PQmakeEmptyPGresult(conn,
+ PGRES_COMMAND_OK);
+ if (!conn->result)
+ {
+ libpq_append_conn_error(conn, "out of memory");
+ pqSaveErrorResult(conn);
+ }
+ }
+ conn->asyncStatus = PGASYNC_READY;
+ }
+ break;
case PqMsg_ParameterStatus:
if (getParameterStatus(conn))
return;
diff --git a/src/interfaces/libpq/fe-trace.c b/src/interfaces/libpq/fe-trace.c
index b18e3deab6a..9ff0c0538c2 100644
--- a/src/interfaces/libpq/fe-trace.c
+++ b/src/interfaces/libpq/fe-trace.c
@@ -514,6 +514,23 @@ pqTraceOutputW(FILE *f, const char *message, int *cursor, int length)
pqTraceOutputInt16(f, message, cursor);
}
+/* ParameterSet(F) or ParameterSetComplete(B) */
+static void
+pqTraceOutputU(FILE *f, bool toServer, const char *message, int *cursor)
+{
+ if (toServer)
+ {
+ fprintf(f, "ParameterSet\t");
+ pqTraceOutputString(f, message, cursor, false);
+ pqTraceOutputString(f, message, cursor, false);
+ }
+ else
+ {
+ fprintf(f, "ParameterSetComplete");
+ /* No message content */
+ }
+}
+
/* ReadyForQuery */
static void
pqTraceOutputZ(FILE *f, const char *message, int *cursor)
@@ -589,6 +606,10 @@ pqTraceOutputMessage(PGconn *conn, const char *message, bool toServer)
Assert(PqMsg_Close == PqMsg_CommandComplete);
pqTraceOutputC(conn->Pfdebug, toServer, message, &logCursor);
break;
+ case PqMsg_ParameterSet:
+ Assert(PqMsg_ParameterSet == PqMsg_ParameterSetComplete);
+ pqTraceOutputU(conn->Pfdebug, toServer, message, &logCursor);
+ break;
case PqMsg_CopyData:
/* Drop COPY data to reduce the overhead of logging. */
break;
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 408ba495088..e675c0f7f98 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -555,6 +555,9 @@ extern PGresult *PQclosePortal(PGconn *conn, const char *portal);
extern int PQsendClosePrepared(PGconn *conn, const char *stmt);
extern int PQsendClosePortal(PGconn *conn, const char *portal);
+extern PGresult *PQparameterSet(PGconn *conn, const char *parameter, const char *value);
+extern int PQsendParameterSet(PGconn *conn, const char *parameter, const char *value);
+
/* Delete a PGresult */
extern void PQclear(PGresult *res);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index c379391a6b2..12679d035f5 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -322,7 +322,8 @@ typedef enum
PGQUERY_PREPARE, /* Parse only (PQprepare) */
PGQUERY_DESCRIBE, /* Describe Statement or Portal */
PGQUERY_SYNC, /* Sync (at end of a pipeline) */
- PGQUERY_CLOSE /* Close Statement or Portal */
+ PGQUERY_CLOSE, /* Close Statement or Portal */
+ PGQUERY_PARAMETER_SET /* Set a server parameter */
} PGQueryClass;
/*
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index 3c009ee1539..5d223965479 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -1046,6 +1046,160 @@ test_prepared(PGconn *conn)
fprintf(stderr, "ok\n");
}
+static void
+test_parameter_set(PGconn *conn)
+{
+ PGresult *res = NULL;
+ const char *val;
+
+ /* Outside of a pipeline */
+ if (PQsendParameterSet(conn, "work_mem", "42MB") != 1)
+ pg_fatal("PQsendClosePortal failed: %s", PQerrorMessage(conn));
+ res = PQgetResult(conn);
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("failed to set parameter: %s", PQerrorMessage(conn));
+ PQclear(res);
+ res = PQexec(conn, "SHOW work_mem");
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pg_fatal("Expected tuples, got %s: %s",
+ PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));
+ if (PQntuples(res) != 1)
+ pg_fatal("expected 1 result, got %d", PQntuples(res));
+
+ val = PQgetvalue(res, 0, 0);
+ if (strcmp(val, "42MB") != 0)
+ pg_fatal("expected 42MB, got %s", val);
+ PQclear(res);
+
+ /* In a pipeline */
+ if (PQenterPipelineMode(conn) != 1)
+ pg_fatal("failed to enter pipeline mode: %s", PQerrorMessage(conn));
+ if (PQsendParameterSet(conn, "work_mem", "10MB") != 1)
+ pg_fatal("PQsendClosePortal failed: %s", PQerrorMessage(conn));
+ PQsendFlushRequest(conn);
+ res = PQgetResult(conn);
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("failed to set parameter: %s", PQerrorMessage(conn));
+ PQclear(res);
+ res = PQgetResult(conn);
+ if (res != NULL)
+ pg_fatal("did not receive terminating NULL");
+ if (PQsendQueryParams(conn, "SHOW work_mem", 0, NULL, NULL, NULL, NULL, 0) != 1)
+ pg_fatal("failed to send query: %s", PQerrorMessage(conn));
+ PQsendFlushRequest(conn);
+ res = PQgetResult(conn);
+ if (res == NULL)
+ pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
+ PQerrorMessage(conn));
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pg_fatal("unexpected result code %s from first pipeline item",
+ PQresStatus(PQresultStatus(res)));
+
+ val = PQgetvalue(res, 0, 0);
+ if (strcmp(val, "10MB") != 0)
+ pg_fatal("expected 10MB, got %s", val);
+ PQclear(res);
+ if (PQexitPipelineMode(conn) != 1)
+ pg_fatal("exiting pipeline failed: %s", PQerrorMessage(conn));
+
+ /* In blocking mode */
+ res = PQparameterSet(conn, "work_mem", "42MB");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("failed to set parameter: %s", PQerrorMessage(conn));
+ PQclear(res);
+ res = PQexec(conn, "SHOW work_mem");
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pg_fatal("Expected tuples, got %s: %s",
+ PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));
+ if (PQntuples(res) != 1)
+ pg_fatal("expected 1 result, got %d", PQntuples(res));
+
+ val = PQgetvalue(res, 0, 0);
+ if (strcmp(val, "42MB") != 0)
+ pg_fatal("expected 42MB, got %s", val);
+ PQclear(res);
+
+ /* In a failed transaction */
+
+ res = PQexec(conn, "BEGIN");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("failed to begin transaction: %s", PQerrorMessage(conn));
+
+ res = PQexec(conn, "SELECT 0/0");
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ pg_fatal("PQexec should have failed with division by zero");
+
+ res = PQparameterSet(conn, "work_mem", "10MB");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("failed to set parameter: %s", PQerrorMessage(conn));
+ res = PQexec(conn, "ROLLBACK");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("failed to set parameter: %s", PQerrorMessage(conn));
+
+ res = PQexec(conn, "SHOW work_mem");
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pg_fatal("Expected tuples, got %s: %s",
+ PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));
+ val = PQgetvalue(res, 0, 0);
+ if (strcmp(val, "10MB") != 0)
+ pg_fatal("expected 10MB, got %s", val);
+ PQclear(res);
+
+ /* Fails a transaction */
+ res = PQexec(conn, "BEGIN");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("failed to begin transaction: %s", PQerrorMessage(conn));
+
+ res = PQparameterSet(conn, "work_mem", "doesnotwork");
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ pg_fatal("failed to set parameter: %s", PQerrorMessage(conn));
+
+ res = PQexec(conn, "SELECT 1");
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ pg_fatal("PQexec should have failed with 'current transaction is aborted'");
+
+ res = PQexec(conn, "ROLLBACK");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("failed to set parameter: %s", PQerrorMessage(conn));
+
+ /* In a failed pipeline */
+ if (PQenterPipelineMode(conn) != 1)
+ pg_fatal("failed to enter pipeline mode: %s", PQerrorMessage(conn));
+ if (PQsendQueryParams(conn, "SELECT 0/0", 0, NULL, NULL, NULL, NULL, 0) != 1)
+ pg_fatal("failed to send query: %s", PQerrorMessage(conn));
+ if (PQsendParameterSet(conn, "work_mem", "12MB") != 1)
+ pg_fatal("PQsendClosePortal failed: %s", PQerrorMessage(conn));
+ if (PQpipelineSync(conn) != 1)
+ pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
+ res = PQgetResult(conn);
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ pg_fatal("PQexec should have failed with division by zero");
+ res = PQgetResult(conn);
+ if (res != NULL)
+ pg_fatal("did not receive terminating NULL");
+ res = PQgetResult(conn);
+ if (PQresultStatus(res) != PGRES_PIPELINE_ABORTED)
+ pg_fatal("pipeline was not aborted");
+ res = PQgetResult(conn);
+ if (res != NULL)
+ pg_fatal("did not receive terminating NULL");
+ res = PQgetResult(conn);
+ if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
+ pg_fatal("Unexpected result code %s instead of PGRES_PIPELINE_SYNC, error: %s",
+ PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));
+ if (PQexitPipelineMode(conn) != 1)
+ pg_fatal("exiting pipeline failed: %s", PQerrorMessage(conn));
+
+ res = PQexec(conn, "SHOW work_mem");
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pg_fatal("Expected tuples, got %s: %s",
+ PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));
+ val = PQgetvalue(res, 0, 0);
+ if (strcmp(val, "10MB") != 0)
+ pg_fatal("expected 10MB, got %s", val);
+ PQclear(res);
+}
+
/* Notice processor: print notices, and count how many we got */
static void
notice_processor(void *arg, const char *message)
@@ -1749,6 +1903,7 @@ print_test_list(void)
printf("disallowed_in_pipeline\n");
printf("multi_pipelines\n");
printf("nosync\n");
+ printf("parameter_set\n");
printf("pipeline_abort\n");
printf("pipeline_idle\n");
printf("pipelined_insert\n");
@@ -1853,6 +2008,8 @@ main(int argc, char **argv)
test_multi_pipelines(conn);
else if (strcmp(testname, "nosync") == 0)
test_nosync(conn);
+ else if (strcmp(testname, "parameter_set") == 0)
+ test_parameter_set(conn);
else if (strcmp(testname, "pipeline_abort") == 0)
test_pipeline_abort(conn);
else if (strcmp(testname, "pipeline_idle") == 0)
diff --git a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl b/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
index 71a11ddf259..173478548ed 100644
--- a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
+++ b/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
@@ -37,7 +37,7 @@ for my $testname (@tests)
{
my @extraargs = ('-r', $numrows);
my $cmptrace = grep(/^$testname$/,
- qw(simple_pipeline nosync multi_pipelines prepared singlerow
+ qw(simple_pipeline nosync multi_pipelines parameter_set prepared singlerow
pipeline_abort pipeline_idle transaction
disallowed_in_pipeline)) > 0;
diff --git a/src/test/modules/libpq_pipeline/traces/parameter_set.trace b/src/test/modules/libpq_pipeline/traces/parameter_set.trace
new file mode 100644
index 00000000000..8f42d398476
--- /dev/null
+++ b/src/test/modules/libpq_pipeline/traces/parameter_set.trace
@@ -0,0 +1,77 @@
+F 18 ParameterSet "work_mem" "42MB"
+F 4 Sync
+B 4 ParameterSetComplete
+B 5 ReadyForQuery I
+F 18 Query "SHOW work_mem"
+B 33 RowDescription 1 "work_mem" NNNN 0 NNNN 65535 -1 0
+B 14 DataRow 1 4 '42MB'
+B 9 CommandComplete "SHOW"
+B 5 ReadyForQuery I
+F 18 ParameterSet "work_mem" "10MB"
+F 4 Flush
+B 4 ParameterSetComplete
+F 21 Parse "" "SHOW work_mem" 0
+F 14 Bind "" "" 0 0 1 0
+F 6 Describe P ""
+F 9 Execute "" 0
+F 4 Flush
+B 4 ParseComplete
+B 4 BindComplete
+B 33 RowDescription 1 "work_mem" NNNN 0 NNNN 65535 -1 0
+B 14 DataRow 1 4 '10MB'
+B 9 CommandComplete "SHOW"
+F 18 ParameterSet "work_mem" "42MB"
+F 4 Sync
+B 4 ParameterSetComplete
+B 5 ReadyForQuery I
+F 18 Query "SHOW work_mem"
+B 33 RowDescription 1 "work_mem" NNNN 0 NNNN 65535 -1 0
+B 14 DataRow 1 4 '42MB'
+B 9 CommandComplete "SHOW"
+B 5 ReadyForQuery I
+F 10 Query "BEGIN"
+B 10 CommandComplete "BEGIN"
+B 5 ReadyForQuery T
+F 15 Query "SELECT 0/0"
+B NN ErrorResponse S "ERROR" V "ERROR" C "22012" M "division by zero" F "SSSS" L "SSSS" R "SSSS" \x00
+B 5 ReadyForQuery E
+F 18 ParameterSet "work_mem" "10MB"
+F 4 Sync
+B 4 ParameterSetComplete
+B 5 ReadyForQuery E
+F 13 Query "ROLLBACK"
+B 13 CommandComplete "ROLLBACK"
+B 5 ReadyForQuery I
+F 18 Query "SHOW work_mem"
+B 33 RowDescription 1 "work_mem" NNNN 0 NNNN 65535 -1 0
+B 14 DataRow 1 4 '10MB'
+B 9 CommandComplete "SHOW"
+B 5 ReadyForQuery I
+F 10 Query "BEGIN"
+B 10 CommandComplete "BEGIN"
+B 5 ReadyForQuery T
+F 25 ParameterSet "work_mem" "doesnotwork"
+F 4 Sync
+B NN ErrorResponse S "ERROR" V "ERROR" C "22023" M "invalid value for parameter "work_mem": "doesnotwork"" F "SSSS" L "SSSS" R "SSSS" \x00
+B 5 ReadyForQuery E
+F 13 Query "SELECT 1"
+B NN ErrorResponse S "ERROR" V "ERROR" C "25P02" M "current transaction is aborted, commands ignored until end of transaction block" F "SSSS" L "SSSS" R "SSSS" \x00
+B 5 ReadyForQuery E
+F 13 Query "ROLLBACK"
+B 13 CommandComplete "ROLLBACK"
+B 5 ReadyForQuery I
+F 18 Parse "" "SELECT 0/0" 0
+F 14 Bind "" "" 0 0 1 0
+F 6 Describe P ""
+F 9 Execute "" 0
+F 18 ParameterSet "work_mem" "12MB"
+F 4 Sync
+B 4 ParseComplete
+B NN ErrorResponse S "ERROR" V "ERROR" C "22012" M "division by zero" F "SSSS" L "SSSS" R "SSSS" \x00
+B 5 ReadyForQuery I
+F 18 Query "SHOW work_mem"
+B 33 RowDescription 1 "work_mem" NNNN 0 NNNN 65535 -1 0
+B 14 DataRow 1 4 '10MB'
+B 9 CommandComplete "SHOW"
+B 5 ReadyForQuery I
+F 4 Terminate
--
2.34.1
From 22b3a872d5397221e3d61690e13ab5054437eec0 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Tue, 2 Jan 2024 12:07:35 +0100
Subject: [PATCH v2 2/4] libpq: Include minor version number in
PQprotocolVersion
The return value from PQprotocolVersion so far only returned the major
version number. This wasn't an issue practice before because we never
had any protocol versions because the minor version was always zero. But
it has become an issue now because we want to bump the minor protocol
version in a future commit. This commit changes the return value to have
the same format as PQserverVersion, i.e. we multiply the major version
by 10000 and add the minor version.
To make sure we return the same value for each protocol version across
libpq versions we only do this calculation for protocol versions above
3.0. For protocol version 3.0 we continue returning a plain 3.
---
doc/src/sgml/libpq.sgml | 12 ++++++------
src/include/libpq/pqcomm.h | 1 +
src/interfaces/libpq/fe-connect.c | 4 +++-
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 2e9ae41e389..72b66295f69 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2566,12 +2566,12 @@ const char *PQparameterStatus(const PGconn *conn, const char *paramName);
int PQprotocolVersion(const PGconn *conn);
</synopsis>
Applications might wish to use this function to determine whether certain
- features are supported. Currently, the possible values are 3
- (3.0 protocol), or zero (connection bad). The protocol version will
- not change after connection startup is complete, but it could
- theoretically change during a connection reset. The 3.0 protocol is
- supported by <productname>PostgreSQL</productname> server versions 7.4
- and above.
+ features are supported. The result is formed by multiplying the server's
+ major version number by 10000 and adding the minor version number. For
+ example, version 3.1 will be returned as 30001, and version 4.0 will
+ be returned as 40000. The exception to this rule is protocol version
+ 3.0, which will be returned as the value 3 for backwards compatibility
+ reasons. Zero is returned if the connection is bad.
</para>
</listitem>
</varlistentry>
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 46a0946b8b2..999ab94b9c8 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -86,6 +86,7 @@ is_unixsock_path(const char *path)
#define PG_PROTOCOL_MAJOR(v) ((v) >> 16)
#define PG_PROTOCOL_MINOR(v) ((v) & 0x0000ffff)
+#define PG_PROTOCOL_FULL(v) (PG_PROTOCOL_MAJOR(v) * 10000 + PG_PROTOCOL_MINOR(v))
#define PG_PROTOCOL(m,n) (((m) << 16) | (n))
/*
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 14214cac62d..aa01443cf03 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -7263,7 +7263,9 @@ PQprotocolVersion(const PGconn *conn)
return 0;
if (conn->status == CONNECTION_BAD)
return 0;
- return PG_PROTOCOL_MAJOR(conn->pversion);
+ if (conn->pversion == PG_PROTOCOL(3, 0))
+ return 3;
+ return PG_PROTOCOL_FULL(conn->pversion);
}
const char **
--
2.34.1
