public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v56 2/6] Add conditional lock feature to dshash
8+ messages / 3 participants
[nested] [flat]
* [PATCH v56 2/6] Add conditional lock feature to dshash
@ 2020-03-13 07:58 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Kyotaro Horiguchi @ 2020-03-13 07:58 UTC (permalink / raw)
Dshash currently waits for lock unconditionally. It is inconvenient
when we want to avoid being blocked by other processes. This commit
adds alternative functions of dshash_find and dshash_find_or_insert
that allows immediate return on lock failure.
---
src/backend/lib/dshash.c | 98 +++++++++++++++++++++-------------------
src/include/lib/dshash.h | 3 ++
2 files changed, 55 insertions(+), 46 deletions(-)
diff --git a/src/backend/lib/dshash.c b/src/backend/lib/dshash.c
index 29ad767618..f79b6de245 100644
--- a/src/backend/lib/dshash.c
+++ b/src/backend/lib/dshash.c
@@ -383,6 +383,10 @@ dshash_get_hash_table_handle(dshash_table *hash_table)
* the caller must take care to ensure that the entry is not left corrupted.
* The lock mode is either shared or exclusive depending on 'exclusive'.
*
+ * If found is not NULL, *found is set to true if the key is found in the hash
+ * table. If the key is not found, *found is set to false and a pointer to a
+ * newly created entry is returned.
+ *
* The caller must not lock a lock already.
*
* Note that the lock held is in fact an LWLock, so interrupts will be held on
@@ -392,36 +396,7 @@ dshash_get_hash_table_handle(dshash_table *hash_table)
void *
dshash_find(dshash_table *hash_table, const void *key, bool exclusive)
{
- dshash_hash hash;
- size_t partition;
- dshash_table_item *item;
-
- hash = hash_key(hash_table, key);
- partition = PARTITION_FOR_HASH(hash);
-
- Assert(hash_table->control->magic == DSHASH_MAGIC);
- Assert(!hash_table->find_locked);
-
- LWLockAcquire(PARTITION_LOCK(hash_table, partition),
- exclusive ? LW_EXCLUSIVE : LW_SHARED);
- ensure_valid_bucket_pointers(hash_table);
-
- /* Search the active bucket. */
- item = find_in_bucket(hash_table, key, BUCKET_FOR_HASH(hash_table, hash));
-
- if (!item)
- {
- /* Not found. */
- LWLockRelease(PARTITION_LOCK(hash_table, partition));
- return NULL;
- }
- else
- {
- /* The caller will free the lock by calling dshash_release_lock. */
- hash_table->find_locked = true;
- hash_table->find_exclusively_locked = exclusive;
- return ENTRY_FROM_ITEM(item);
- }
+ return dshash_find_extended(hash_table, key, exclusive, false, false, NULL);
}
/*
@@ -439,31 +414,60 @@ dshash_find_or_insert(dshash_table *hash_table,
const void *key,
bool *found)
{
- dshash_hash hash;
- size_t partition_index;
- dshash_partition *partition;
+ return dshash_find_extended(hash_table, key, true, false, true, found);
+}
+
+
+/*
+ * Find the key in the hash table.
+ *
+ * "exclusive" is the lock mode in which the partition for the returned item
+ * is locked. If "nowait" is true, the function immediately returns if
+ * required lock was not acquired. "insert" indicates insert mode. In this
+ * mode new entry is inserted and set *found to false. *found is set to true if
+ * found. "found" must be non-null in this mode.
+ */
+void *
+dshash_find_extended(dshash_table *hash_table, const void *key,
+ bool exclusive, bool nowait, bool insert, bool *found)
+{
+ dshash_hash hash = hash_key(hash_table, key);
+ size_t partidx = PARTITION_FOR_HASH(hash);
+ dshash_partition *partition = &hash_table->control->partitions[partidx];
+ LWLockMode lockmode = exclusive ? LW_EXCLUSIVE : LW_SHARED;
dshash_table_item *item;
- hash = hash_key(hash_table, key);
- partition_index = PARTITION_FOR_HASH(hash);
- partition = &hash_table->control->partitions[partition_index];
-
- Assert(hash_table->control->magic == DSHASH_MAGIC);
- Assert(!hash_table->find_locked);
+ /* must be exclusive when insert allowed */
+ Assert(!insert || (exclusive && found != NULL));
restart:
- LWLockAcquire(PARTITION_LOCK(hash_table, partition_index),
- LW_EXCLUSIVE);
+ if (!nowait)
+ LWLockAcquire(PARTITION_LOCK(hash_table, partidx), lockmode);
+ else if (!LWLockConditionalAcquire(PARTITION_LOCK(hash_table, partidx),
+ lockmode))
+ return NULL;
+
ensure_valid_bucket_pointers(hash_table);
/* Search the active bucket. */
item = find_in_bucket(hash_table, key, BUCKET_FOR_HASH(hash_table, hash));
if (item)
- *found = true;
+ {
+ if (found)
+ *found = true;
+ }
else
{
- *found = false;
+ if (found)
+ *found = false;
+
+ if (!insert)
+ {
+ /* The caller didn't told to add a new entry. */
+ LWLockRelease(PARTITION_LOCK(hash_table, partidx));
+ return NULL;
+ }
/* Check if we are getting too full. */
if (partition->count > MAX_COUNT_PER_PARTITION(hash_table))
@@ -479,7 +483,8 @@ restart:
* Give up our existing lock first, because resizing needs to
* reacquire all the locks in the right order to avoid deadlocks.
*/
- LWLockRelease(PARTITION_LOCK(hash_table, partition_index));
+ LWLockRelease(PARTITION_LOCK(hash_table, partidx));
+
resize(hash_table, hash_table->size_log2 + 1);
goto restart;
@@ -493,12 +498,13 @@ restart:
++partition->count;
}
- /* The caller must release the lock with dshash_release_lock. */
+ /* The caller will free the lock by calling dshash_release_lock. */
hash_table->find_locked = true;
- hash_table->find_exclusively_locked = true;
+ hash_table->find_exclusively_locked = exclusive;
return ENTRY_FROM_ITEM(item);
}
+
/*
* Remove an entry by key. Returns true if the key was found and the
* corresponding entry was removed.
diff --git a/src/include/lib/dshash.h b/src/include/lib/dshash.h
index a6ea377173..5b8114d041 100644
--- a/src/include/lib/dshash.h
+++ b/src/include/lib/dshash.h
@@ -91,6 +91,9 @@ extern void *dshash_find(dshash_table *hash_table,
const void *key, bool exclusive);
extern void *dshash_find_or_insert(dshash_table *hash_table,
const void *key, bool *found);
+extern void *dshash_find_extended(dshash_table *hash_table, const void *key,
+ bool exclusive, bool nowait, bool insert,
+ bool *found);
extern bool dshash_delete_key(dshash_table *hash_table, const void *key);
extern void dshash_delete_entry(dshash_table *hash_table, void *entry);
extern void dshash_release_lock(dshash_table *hash_table, void *entry);
--
2.27.0
----Next_Part(Tue_Mar_16_10_27_55_2021_500)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v56-0003-Shared-memory-based-stats-collector.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Deleting prepared statements from libpq.
@ 2023-06-23 03:59 Michael Paquier <[email protected]>
2023-06-23 07:39 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Michael Paquier @ 2023-06-23 03:59 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: jian he <[email protected]>; Dmitry Igrishin <[email protected]>; pgsql-hackers; Daniele Varrazzo <[email protected]>
On Tue, Jun 20, 2023 at 01:42:13PM +0200, Jelte Fennema wrote:
Thanks for updating the patch.
> On Tue, 20 Jun 2023 at 06:18, Michael Paquier <[email protected]> wrote:
>> The amount of duplication between the describe and close paths
>> concerns me a bit. Should PQsendClose() and PQsendDescribe() be
>> merged into a single routine (say PQsendCommand), that uses a message
>> type for pqPutMsgStart and a queryclass as extra arguments?
>
> Done (I called it PQsendTypedCommand)
Okay by me to use this name.
I have a few comments about the tests. The docs and the code seem to
be in line.
+ if (PQsendClosePrepared(conn, "select_one") != 1)
+ pg_fatal("PQsendClosePortal failed: %s", PQerrorMessage(conn));
+ if (PQpipelineSync(conn) != 1)
+ pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
+
+ res = PQgetResult(conn);
+ if (res == NULL)
+ pg_fatal("expected non-NULL result");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
+ PQclear(res);
+ res = PQgetResult(conn);
+ if (res != NULL)
+ pg_fatal("expected NULL result");
+ res = PQgetResult(conn);
+ if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
+ pg_fatal("expected PGRES_PIPELINE_SYNC, got %s", PQresStatus(PQresultStatus(res)));
Okay, so here this checks that a PQresult is returned on the first
call, and that NULL is returned on the second call. Okay with that.
Perhaps this should add a fprintf(stderr, "closing statement..") at
the top of the test block? That's a minor point.
+ /*
+ * Also test the blocking close, this should not fail since closing a
+ * non-existent prepared statement is a no-op
+ */
+ res = PQclosePrepared(conn, "select_one");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
[...]
res = PQgetResult(conn);
if (res == NULL)
- pg_fatal("expected NULL result");
+ pg_fatal("expected non-NULL result");
This should check for the NULL-ness of the result returned for
PQclosePrepared() rather than changing the behavior of the follow-up
calls?
+ if (PQsendClosePortal(conn, "cursor_one") != 1)
+ pg_fatal("PQsendClosePortal failed: %s", PQerrorMessage(conn));
+ if (PQpipelineSync(conn) != 1)
+ pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
Perhaps I would add an extra fprint about the portal closing test,
just for clarity of the test flow.
@@ -2534,6 +2615,7 @@ sendFailed:
return 0;
}
+
/*
Noise diff.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Deleting prepared statements from libpq.
2023-06-23 03:59 Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
@ 2023-06-23 07:39 ` Jelte Fennema <[email protected]>
2023-06-23 07:53 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Jelte Fennema @ 2023-06-23 07:39 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: jian he <[email protected]>; Dmitry Igrishin <[email protected]>; pgsql-hackers; Daniele Varrazzo <[email protected]>
On Fri, 23 Jun 2023 at 05:59, Michael Paquier <[email protected]> wrote:
> [...]
> res = PQgetResult(conn);
> if (res == NULL)
> - pg_fatal("expected NULL result");
> + pg_fatal("expected non-NULL result");
>
> This should check for the NULL-ness of the result returned for
> PQclosePrepared() rather than changing the behavior of the follow-up
> calls?
To be clear, it didn't actually change the behaviour. I only changed
the error message, since it said the exact opposite of what it was
expecting. I split this minor fix into its own commit now to clarify
that. I think it would even make sense to commit this small patch to
the PG16 branch, since it's a bugfix in the tests (and possibly even
back-patch to others if that's not a lot of work). I changed the error
message to be in line with one from earlier in the test.
I addressed all of your other comments.
Attachments:
[application/octet-stream] v6-0002-Support-sending-Close-messages-from-libpq.patch (22.1K, ../../CAGECzQTkShHecFF+EZrm94Lbsu2ej569T=bz+PjMbw9Aiioxuw@mail.gmail.com/2-v6-0002-Support-sending-Close-messages-from-libpq.patch)
download | inline diff:
From 187ac78b911c7f1c55da582f38fe4cd1ec1318a8 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Fri, 16 Jun 2023 17:15:48 +0200
Subject: [PATCH v6 2/2] Support sending Close messages from libpq
This part of the protocol did not have a libpq implementation yet. That
is a shame because connection poolers can much more easily intercept
these Close messages than a DEALLOCATE query. Odyssey has prepared
statement support implemented using the Close message, and PgBouncer is
currently trying to do the same. But libpq based clients are not able
to use this feature of these connection poolers because they cannot send
the Close protocol message.
---
doc/src/sgml/libpq.sgml | 125 ++++++++++++++++--
src/interfaces/libpq/exports.txt | 4 +
src/interfaces/libpq/fe-exec.c | 123 ++++++++++++++---
src/interfaces/libpq/fe-protocol3.c | 19 ++-
src/interfaces/libpq/libpq-fe.h | 6 +
src/interfaces/libpq/libpq-int.h | 2 +-
.../modules/libpq_pipeline/libpq_pipeline.c | 67 +++++++++-
.../libpq_pipeline/traces/prepared.trace | 24 ++++
8 files changed, 337 insertions(+), 33 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 2225e4e0ef3..3c17b097540 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -3250,10 +3250,7 @@ PGresult *PQprepare(PGconn *conn,
Prepared statements for use with <xref linkend="libpq-PQexecPrepared"/> can also
be created by executing SQL <xref linkend="sql-prepare"/>
- statements. Also, although there is no <application>libpq</application>
- function for deleting a prepared statement, the SQL <xref
- linkend="sql-deallocate"/> statement
- can be used for that purpose.
+ statements.
</para>
<para>
@@ -3360,6 +3357,66 @@ PGresult *PQdescribePortal(PGconn *conn, const char *portalName);
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="libpq-PQclosePrepared">
+ <term><function>PQclosePrepared</function><indexterm><primary>PQclosePrepared</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ Submits a request to close the specified prepared statement, and waits
+ for completion.
+<synopsis>
+PGresult *PQclosePrepared(PGconn *conn, const char *stmtName);
+</synopsis>
+ </para>
+
+ <para>
+ <xref linkend="libpq-PQclosePrepared"/> allows an application to close
+ a previously prepared statement. Closing a statement releases all
+ of its associated resources on the server and allows its name to be
+ reused.
+ </para>
+
+ <para>
+ <parameter>stmtName</parameter> can be <literal>""</literal> or
+ <symbol>NULL</symbol> to reference the unnamed statement. It is fine
+ if no statement exists with this name, in that case the operation is a
+ no-op. On success, a <structname>PGresult</structname> with
+ status <literal>PGRES_COMMAND_OK</literal> is returned.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQclosePortal">
+ <term><function>PQclosePortal</function><indexterm><primary>PQclosePortal</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ Submits a request to close the specified portal, and waits for
+ completion.
+<synopsis>
+PGresult *PQclosePortal(PGconn *conn, const char *portalName);
+</synopsis>
+ </para>
+
+ <para>
+ <xref linkend="libpq-PQclosePortal"/> allows an application to trigger
+ a close of a previously created portal. Closing a portal releases all
+ of its associated resources on the server and allows its name to be
+ reused. (<application>libpq</application> does not provide any
+ direct access to portals, but you can use this function to close a
+ cursor created with a <command>DECLARE CURSOR</command> SQL command.)
+ </para>
+
+ <para>
+ <parameter>portalName</parameter> can be <literal>""</literal> or
+ <symbol>NULL</symbol> to reference the unnamed portal. It is fine
+ if no portal exists with this name, in that case the operation is a
+ no-op. On success, a <structname>PGresult</structname> with status
+ <literal>PGRES_COMMAND_OK</literal> is returned.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
@@ -4851,15 +4908,19 @@ unsigned char *PQunescapeBytea(const unsigned char *from, size_t *to_length);
<xref linkend="libpq-PQsendQueryParams"/>,
<xref linkend="libpq-PQsendPrepare"/>,
<xref linkend="libpq-PQsendQueryPrepared"/>,
- <xref linkend="libpq-PQsendDescribePrepared"/>, and
+ <xref linkend="libpq-PQsendDescribePrepared"/>,
<xref linkend="libpq-PQsendDescribePortal"/>,
+ <xref linkend="libpq-PQsendClosePrepared"/>, and
+ <xref linkend="libpq-PQsendClosePortal"/>,
which can be used with <xref linkend="libpq-PQgetResult"/> to duplicate
the functionality of
<xref linkend="libpq-PQexecParams"/>,
<xref linkend="libpq-PQprepare"/>,
<xref linkend="libpq-PQexecPrepared"/>,
- <xref linkend="libpq-PQdescribePrepared"/>, and
+ <xref linkend="libpq-PQdescribePrepared"/>,
<xref linkend="libpq-PQdescribePortal"/>
+ <xref linkend="libpq-PQclosePrepared"/>, and
+ <xref linkend="libpq-PQclosePortal"/>
respectively.
<variablelist>
@@ -5008,6 +5069,46 @@ int PQsendDescribePortal(PGconn *conn, const char *portalName);
</listitem>
</varlistentry>
+ <varlistentry id="libpq-PQsendClosePrepared">
+ <term><function>PQsendClosePrepared</function><indexterm><primary>PQsendClosePrepared</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ Submits a request to close the specified prepared statement, without
+ waiting for completion.
+<synopsis>
+int PQsendClosePrepared(PGconn *conn, const char *stmtName);
+</synopsis>
+
+ This is an asynchronous version of <xref linkend="libpq-PQclosePrepared"/>:
+ it returns 1 if it was able to dispatch the request, and 0 if not.
+ After a successful call, call <xref linkend="libpq-PQgetResult"/> to
+ obtain the results. The function's parameters are handled
+ identically to <xref linkend="libpq-PQclosePrepared"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQsendClosePortal">
+ <term><function>PQsendClosePortal</function><indexterm><primary>PQsendClosePortal</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ Submits a request to close specified portal, without waiting for
+ completion.
+<synopsis>
+int PQsendClosePortal(PGconn *conn, const char *portalName);
+</synopsis>
+
+ This is an asynchronous version of <xref linkend="libpq-PQclosePortal"/>:
+ it returns 1 if it was able to dispatch the request, and 0 if not.
+ After a successful call, call <xref linkend="libpq-PQgetResult"/> to
+ obtain the results. The function's parameters are handled
+ identically to <xref linkend="libpq-PQclosePortal"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-PQgetResult">
<term><function>PQgetResult</function><indexterm><primary>PQgetResult</primary></indexterm></term>
@@ -5019,7 +5120,9 @@ int PQsendDescribePortal(PGconn *conn, const char *portalName);
<xref linkend="libpq-PQsendPrepare"/>,
<xref linkend="libpq-PQsendQueryPrepared"/>,
<xref linkend="libpq-PQsendDescribePrepared"/>,
- <xref linkend="libpq-PQsendDescribePortal"/>, or
+ <xref linkend="libpq-PQsendDescribePortal"/>,
+ <xref linkend="libpq-PQsendClosePrepared"/>,
+ <xref linkend="libpq-PQsendClosePortal"/>, or
<xref linkend="libpq-PQpipelineSync"/>
call, and returns it.
A null pointer is returned when the command is complete and there
@@ -5350,6 +5453,8 @@ int PQflush(PGconn *conn);
<function>PQexecPrepared</function>,
<function>PQdescribePrepared</function>,
<function>PQdescribePortal</function>,
+ <function>PQclosePrepared</function>,
+ <function>PQclosePortal</function>,
is an error condition.
<function>PQsendQuery</function> is
also disallowed, because it uses the simple query protocol.
@@ -5389,8 +5494,10 @@ int PQflush(PGconn *conn);
establish a synchronization point in the pipeline,
or when <xref linkend="libpq-PQflush"/> is called.
The functions <xref linkend="libpq-PQsendPrepare"/>,
- <xref linkend="libpq-PQsendDescribePrepared"/>, and
- <xref linkend="libpq-PQsendDescribePortal"/> also work in pipeline mode.
+ <xref linkend="libpq-PQsendDescribePrepared"/>,
+ <xref linkend="libpq-PQsendDescribePortal"/>,
+ <xref linkend="libpq-PQsendClosePrepared"/>, and
+ <xref linkend="libpq-PQsendClosePortal"/> also work in pipeline mode.
Result processing is described below.
</para>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 7ded77aff37..850734ac96c 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -187,3 +187,7 @@ PQsetTraceFlags 184
PQmblenBounded 185
PQsendFlushRequest 186
PQconnectionUsedGSSAPI 187
+PQclosePrepared 188
+PQclosePortal 189
+PQsendClosePrepared 190
+PQsendClosePortal 191
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 14d706efd57..dd8b791764c 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -77,8 +77,8 @@ static void parseInput(PGconn *conn);
static PGresult *getCopyResult(PGconn *conn, ExecStatusType copytype);
static bool PQexecStart(PGconn *conn);
static PGresult *PQexecFinish(PGconn *conn);
-static int PQsendDescribe(PGconn *conn, char desc_type,
- const char *desc_target);
+static int PQsendTypedCommand(PGconn *conn, char command, char type,
+ const char *target);
static int check_field_number(const PGresult *res, int field_num);
static void pqPipelineProcessQueue(PGconn *conn);
static int pqPipelineFlush(PGconn *conn);
@@ -2422,7 +2422,7 @@ PQdescribePrepared(PGconn *conn, const char *stmt)
{
if (!PQexecStart(conn))
return NULL;
- if (!PQsendDescribe(conn, 'S', stmt))
+ if (!PQsendTypedCommand(conn, 'D', 'S', stmt))
return NULL;
return PQexecFinish(conn);
}
@@ -2441,7 +2441,7 @@ PQdescribePortal(PGconn *conn, const char *portal)
{
if (!PQexecStart(conn))
return NULL;
- if (!PQsendDescribe(conn, 'P', portal))
+ if (!PQsendTypedCommand(conn, 'D', 'P', portal))
return NULL;
return PQexecFinish(conn);
}
@@ -2456,7 +2456,7 @@ PQdescribePortal(PGconn *conn, const char *portal)
int
PQsendDescribePrepared(PGconn *conn, const char *stmt)
{
- return PQsendDescribe(conn, 'S', stmt);
+ return PQsendTypedCommand(conn, 'D', 'S', stmt);
}
/*
@@ -2469,26 +2469,95 @@ PQsendDescribePrepared(PGconn *conn, const char *stmt)
int
PQsendDescribePortal(PGconn *conn, const char *portal)
{
- return PQsendDescribe(conn, 'P', portal);
+ return PQsendTypedCommand(conn, 'D', 'P', portal);
}
/*
- * PQsendDescribe
- * Common code to send a Describe command
+ * PQclosePrepared
+ * Close a previously prepared statement
*
- * Available options for desc_type are
- * 'S' to describe a prepared statement; or
- * 'P' to describe a portal.
+ * 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 *
+PQclosePrepared(PGconn *conn, const char *stmt)
+{
+ if (!PQexecStart(conn))
+ return NULL;
+ if (!PQsendTypedCommand(conn, 'C', 'S', stmt))
+ return NULL;
+ return PQexecFinish(conn);
+}
+
+/*
+ * PQclosePortal
+ * Close a previously created portal
+ *
+ * This is exactly like PQclosePrepared, but for portals. Note that at the
+ * moment, libpq doesn't really expose portals to the client; but this can be
+ * used with a portal created by a SQL DECLARE CURSOR command.
+ */
+PGresult *
+PQclosePortal(PGconn *conn, const char *portal)
+{
+ if (!PQexecStart(conn))
+ return NULL;
+ if (!PQsendTypedCommand(conn, 'C', 'P', portal))
+ return NULL;
+ return PQexecFinish(conn);
+}
+
+/*
+ * PQsendClosePrepared
+ * Submit a Close Statement command, but don't wait for it to finish
+ *
+ * Returns: 1 if successfully submitted
+ * 0 if error (conn->errorMessage is set)
+ */
+int
+PQsendClosePrepared(PGconn *conn, const char *stmt)
+{
+ return PQsendTypedCommand(conn, 'C', 'S', stmt);
+}
+
+/*
+ * PQsendClosePortal
+ * Submit a Close Portal command, but don't wait for it to finish
+ *
+ * Returns: 1 if successfully submitted
+ * 0 if error (conn->errorMessage is set)
+ */
+int
+PQsendClosePortal(PGconn *conn, const char *portal)
+{
+ return PQsendTypedCommand(conn, 'C', 'P', portal);
+}
+
+/*
+ * PQsendTypedCommand
+ * Common code to send a Describe or Close command
+ *
+ * Available options for command are
+ * 'C' for Close
+ * 'D' for Describe
+ *
+ * Available options for type are
+ * 'S' to a prepared statement; or
+ * 'P' to a portal.
* Returns 1 on success and 0 on failure.
*/
static int
-PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target)
+PQsendTypedCommand(PGconn *conn, char command, char type, const char *target)
{
PGcmdQueueEntry *entry = NULL;
- /* Treat null desc_target as empty string */
- if (!desc_target)
- desc_target = "";
+ /* Treat null target as empty string */
+ if (!target)
+ target = "";
if (!PQsendQueryStart(conn, true))
return 0;
@@ -2497,10 +2566,10 @@ PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target)
if (entry == NULL)
return 0; /* error msg already set */
- /* construct the Describe message */
- if (pqPutMsgStart('D', conn) < 0 ||
- pqPutc(desc_type, conn) < 0 ||
- pqPuts(desc_target, conn) < 0 ||
+ /* construct the Close message */
+ if (pqPutMsgStart(command, conn) < 0 ||
+ pqPutc(type, conn) < 0 ||
+ pqPuts(target, conn) < 0 ||
pqPutMsgEnd(conn) < 0)
goto sendFailed;
@@ -2512,8 +2581,20 @@ PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target)
goto sendFailed;
}
- /* remember we are doing a Describe */
- entry->queryclass = PGQUERY_DESCRIBE;
+ /* remember if we are doing a Close or a Describe */
+ if (command == 'C')
+ {
+ entry->queryclass = PGQUERY_CLOSE;
+ }
+ else if (command == 'D')
+ {
+ entry->queryclass = PGQUERY_DESCRIBE;
+ }
+ else
+ {
+ libpq_append_conn_error(conn, "unknown command type provided");
+ goto sendFailed;
+ }
/*
* Give the data a push (in pipeline mode, only if we're past the size
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 32b66d561cb..7bc6355d17f 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -278,8 +278,25 @@ pqParseInput3(PGconn *conn)
}
break;
case '2': /* Bind Complete */
+ /* Nothing to do for this message type */
+ break;
case '3': /* Close Complete */
- /* Nothing to do for these message types */
+ /* If we're doing PQsendClose, we're done; else ignore */
+ if (conn->cmd_queue_head &&
+ conn->cmd_queue_head->queryclass == PGQUERY_CLOSE)
+ {
+ 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 'S': /* parameter status */
if (getParameterStatus(conn))
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 7476dbe0e90..97762d56f5d 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -548,6 +548,12 @@ extern PGresult *PQdescribePortal(PGconn *conn, const char *portal);
extern int PQsendDescribePrepared(PGconn *conn, const char *stmt);
extern int PQsendDescribePortal(PGconn *conn, const char *portal);
+/* Close prepared statements and portals */
+extern PGresult *PQclosePrepared(PGconn *conn, const char *stmt);
+extern PGresult *PQclosePortal(PGconn *conn, const char *portal);
+extern int PQsendClosePrepared(PGconn *conn, const char *stmt);
+extern int PQsendClosePortal(PGconn *conn, const char *portal);
+
/* 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 0045f83cbfd..cd116f4e95c 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -324,7 +324,7 @@ typedef enum
PGQUERY_PREPARE, /* Parse only (PQprepare) */
PGQUERY_DESCRIBE, /* Describe Statement or Portal */
PGQUERY_SYNC, /* Sync (at end of a pipeline) */
- PGQUERY_CLOSE
+ PGQUERY_CLOSE /* Close Statement or Portal */
} PGQueryClass;
/*
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index 1933b078ebf..a2f5c8c0027 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -896,7 +896,7 @@ test_prepared(PGconn *conn)
Oid expected_oids[4];
Oid typ;
- fprintf(stderr, "prepared... ");
+ fprintf(stderr, "preparing statement... ");
if (PQenterPipelineMode(conn) != 1)
pg_fatal("failed to enter pipeline mode: %s", PQerrorMessage(conn));
@@ -947,9 +947,42 @@ test_prepared(PGconn *conn)
if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
pg_fatal("expected PGRES_PIPELINE_SYNC, got %s", PQresStatus(PQresultStatus(res)));
+ fprintf(stderr, "closing statement..");
+ if (PQsendClosePrepared(conn, "select_one") != 1)
+ pg_fatal("PQsendClosePortal failed: %s", PQerrorMessage(conn));
+ if (PQpipelineSync(conn) != 1)
+ pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
+
+ res = PQgetResult(conn);
+ if (res == NULL)
+ pg_fatal("expected non-NULL result");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
+ PQclear(res);
+ res = PQgetResult(conn);
+ if (res != NULL)
+ pg_fatal("expected NULL result");
+ res = PQgetResult(conn);
+ if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
+ pg_fatal("expected PGRES_PIPELINE_SYNC, got %s", PQresStatus(PQresultStatus(res)));
+
if (PQexitPipelineMode(conn) != 1)
pg_fatal("could not exit pipeline mode: %s", PQerrorMessage(conn));
+ /* Now that it's closed we should get an error when describing */
+ res = PQdescribePrepared(conn, "select_one");
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ pg_fatal("expected FATAL_ERROR, got %s", PQresStatus(PQresultStatus(res)));
+
+ /*
+ * Also test the blocking close, this should not fail since closing a
+ * non-existent prepared statement is a no-op
+ */
+ res = PQclosePrepared(conn, "select_one");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
+
+ fprintf(stderr, "creating portal... ");
PQexec(conn, "BEGIN");
PQexec(conn, "DECLARE cursor_one CURSOR FOR SELECT 1");
PQenterPipelineMode(conn);
@@ -975,9 +1008,41 @@ test_prepared(PGconn *conn)
if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
pg_fatal("expected PGRES_PIPELINE_SYNC, got %s", PQresStatus(PQresultStatus(res)));
+ fprintf(stderr, "closing portal... ");
+ if (PQsendClosePortal(conn, "cursor_one") != 1)
+ pg_fatal("PQsendClosePortal failed: %s", PQerrorMessage(conn));
+ if (PQpipelineSync(conn) != 1)
+ pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
+
+ res = PQgetResult(conn);
+ if (res == NULL)
+ pg_fatal("expected non-NULL result");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
+ PQclear(res);
+ res = PQgetResult(conn);
+ if (res != NULL)
+ pg_fatal("expected NULL result");
+ res = PQgetResult(conn);
+ if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
+ pg_fatal("expected PGRES_PIPELINE_SYNC, got %s", PQresStatus(PQresultStatus(res)));
+
if (PQexitPipelineMode(conn) != 1)
pg_fatal("could not exit pipeline mode: %s", PQerrorMessage(conn));
+ /* Now that it's closed we should get an error when describing */
+ res = PQdescribePortal(conn, "cursor_one");
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ pg_fatal("expected FATAL_ERROR, got %s", PQresStatus(PQresultStatus(res)));
+
+ /*
+ * Also test the blocking close, this should not fail since closing a
+ * non-existent portal is a no-op
+ */
+ res = PQclosePortal(conn, "cursor_one");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
+
fprintf(stderr, "ok\n");
}
diff --git a/src/test/modules/libpq_pipeline/traces/prepared.trace b/src/test/modules/libpq_pipeline/traces/prepared.trace
index 1a7de5c3e65..aeb5de109e0 100644
--- a/src/test/modules/libpq_pipeline/traces/prepared.trace
+++ b/src/test/modules/libpq_pipeline/traces/prepared.trace
@@ -5,6 +5,18 @@ B 4 ParseComplete
B 10 ParameterDescription 1 NNNN
B 113 RowDescription 4 "?column?" NNNN 0 NNNN 4 -1 0 "?column?" NNNN 0 NNNN 65535 -1 0 "numeric" NNNN 0 NNNN 65535 -1 0 "interval" NNNN 0 NNNN 16 -1 0
B 5 ReadyForQuery I
+F 16 Close S "select_one"
+F 4 Sync
+B 4 CloseComplete
+B 5 ReadyForQuery I
+F 16 Describe S "select_one"
+F 4 Sync
+B NN ErrorResponse S "ERROR" V "ERROR" C "26000" M "prepared statement "select_one" does not exist" F "SSSS" L "SSSS" R "SSSS" \x00
+B 5 ReadyForQuery I
+F 16 Close S "select_one"
+F 4 Sync
+B 4 CloseComplete
+B 5 ReadyForQuery I
F 10 Query "BEGIN"
B 10 CommandComplete "BEGIN"
B 5 ReadyForQuery T
@@ -15,4 +27,16 @@ F 16 Describe P "cursor_one"
F 4 Sync
B 33 RowDescription 1 "?column?" NNNN 0 NNNN 4 -1 0
B 5 ReadyForQuery T
+F 16 Close P "cursor_one"
+F 4 Sync
+B 4 CloseComplete
+B 5 ReadyForQuery T
+F 16 Describe P "cursor_one"
+F 4 Sync
+B NN ErrorResponse S "ERROR" V "ERROR" C "34000" M "portal "cursor_one" does not exist" F "SSSS" L "SSSS" R "SSSS" \x00
+B 5 ReadyForQuery E
+F 16 Close P "cursor_one"
+F 4 Sync
+B 4 CloseComplete
+B 5 ReadyForQuery E
F 4 Terminate
--
2.34.1
[application/octet-stream] v6-0001-Correct-error-message-in-test_prepared.patch (1.1K, ../../CAGECzQTkShHecFF+EZrm94Lbsu2ej569T=bz+PjMbw9Aiioxuw@mail.gmail.com/3-v6-0001-Correct-error-message-in-test_prepared.patch)
download | inline diff:
From cb10504f09f767d6bfb12a2d7ca55ac31393f3ad Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Fri, 23 Jun 2023 09:02:11 +0200
Subject: [PATCH v6 1/2] Correct error message in test_prepared
It said it expecting a NULL result, but it was actually expecting the
opposite. This probably originated from a copy-paste mistake.
---
src/test/modules/libpq_pipeline/libpq_pipeline.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index f5b4d4d1ff2..1933b078ebf 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -959,7 +959,7 @@ test_prepared(PGconn *conn)
pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
res = PQgetResult(conn);
if (res == NULL)
- pg_fatal("expected NULL result");
+ pg_fatal("PQgetResult returned null");
if (PQresultStatus(res) != PGRES_COMMAND_OK)
pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
base-commit: c2122aae636d7121d5cdb64ad1444e1df7f69257
--
2.34.1
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Deleting prepared statements from libpq.
2023-06-23 03:59 Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-06-23 07:39 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
@ 2023-06-23 07:53 ` Michael Paquier <[email protected]>
2023-07-03 12:33 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Michael Paquier @ 2023-06-23 07:53 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: jian he <[email protected]>; Dmitry Igrishin <[email protected]>; pgsql-hackers; Daniele Varrazzo <[email protected]>
On Fri, Jun 23, 2023 at 09:39:00AM +0200, Jelte Fennema wrote:
> To be clear, it didn't actually change the behaviour. I only changed
> the error message, since it said the exact opposite of what it was
> expecting. I split this minor fix into its own commit now to clarify
> that. I think it would even make sense to commit this small patch to
> the PG16 branch, since it's a bugfix in the tests (and possibly even
> back-patch to others if that's not a lot of work).
Yes, agreed that this had better be fixed and backpatched. This
avoids some backpatching fuzz, and that's incorrect as it stands.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Deleting prepared statements from libpq.
2023-06-23 03:59 Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-06-23 07:39 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
2023-06-23 07:53 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
@ 2023-07-03 12:33 ` Jelte Fennema <[email protected]>
2023-07-03 23:28 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Jelte Fennema @ 2023-07-03 12:33 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: jian he <[email protected]>; Dmitry Igrishin <[email protected]>; pgsql-hackers; Daniele Varrazzo <[email protected]>
@Michael is anything else needed from my side? If not, I'll mark the
commitfest entry as "Ready For Committer".
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Deleting prepared statements from libpq.
2023-06-23 03:59 Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-06-23 07:39 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
2023-06-23 07:53 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-07-03 12:33 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
@ 2023-07-03 23:28 ` Michael Paquier <[email protected]>
2023-07-04 07:09 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Michael Paquier @ 2023-07-03 23:28 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: jian he <[email protected]>; Dmitry Igrishin <[email protected]>; pgsql-hackers; Daniele Varrazzo <[email protected]>
On Mon, Jul 03, 2023 at 02:33:55PM +0200, Jelte Fennema wrote:
> @Michael is anything else needed from my side? If not, I'll mark the
> commitfest entry as "Ready For Committer".
Sure, feel free. I was planning to look at and play more with it.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../ZKNZqD%[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Deleting prepared statements from libpq.
2023-06-23 03:59 Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-06-23 07:39 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
2023-06-23 07:53 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-07-03 12:33 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
2023-07-03 23:28 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
@ 2023-07-04 07:09 ` Michael Paquier <[email protected]>
2023-07-04 23:39 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Michael Paquier @ 2023-07-04 07:09 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: jian he <[email protected]>; Dmitry Igrishin <[email protected]>; pgsql-hackers; Daniele Varrazzo <[email protected]>
On Tue, Jul 04, 2023 at 08:28:40AM +0900, Michael Paquier wrote:
> Sure, feel free. I was planning to look at and play more with it.
Well, done.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Deleting prepared statements from libpq.
2023-06-23 03:59 Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-06-23 07:39 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
2023-06-23 07:53 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-07-03 12:33 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
2023-07-03 23:28 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-07-04 07:09 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
@ 2023-07-04 23:39 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Michael Paquier @ 2023-07-04 23:39 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: jian he <[email protected]>; Dmitry Igrishin <[email protected]>; pgsql-hackers; Daniele Varrazzo <[email protected]>
On Tue, Jul 04, 2023 at 04:09:43PM +0900, Michael Paquier wrote:
> On Tue, Jul 04, 2023 at 08:28:40AM +0900, Michael Paquier wrote:
>> Sure, feel free. I was planning to look at and play more with it.
>
> Well, done.
For the sake of completeness, as I forgot to send my notes.
+ if (PQsendClosePrepared(conn, "select_one") != 1)
+ pg_fatal("PQsendClosePortal failed: %s", PQerrorMessage(conn));
There was a small copy-pasto here.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2023-07-04 23:39 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-13 07:58 [PATCH v56 2/6] Add conditional lock feature to dshash Kyotaro Horiguchi <[email protected]>
2023-06-23 03:59 Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-06-23 07:39 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
2023-06-23 07:53 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-07-03 12:33 ` Re: Deleting prepared statements from libpq. Jelte Fennema <[email protected]>
2023-07-03 23:28 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-07-04 07:09 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
2023-07-04 23:39 ` Re: Deleting prepared statements from libpq. Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox