On Thu, Feb 20, 2025 at 10:29:33AM +0100, Anthonin Bonnefoy wrote: > Ha yeah, I forgot about the inactive branches. I've added the new > commands and fixed the behaviour.
And I did not notice that it was as simple as forcing the status in the routines for the new meta-commands, as we do for the existing ones. Noted. > This looks more like an issue on libpq's side as there's no way to > reset or advance the errorReported from ExecQueryAndProcessResults > (plus PQerrorMessage seems to ignore errorReported). I've added an > additional test to track this behaviour for now as this would probably > be better discussed in a dedicated thread. I am not sure if we should change that, actually, as it does not feel completely wrong to stack these errors. That's a bit confusing, sure. Perhaps a new libpq API to retrieve stacked errors when we are in pipeline mode would be more adapted? The design would be different. Anyway, I've stared at the result processing code for a couple of hours, and the branches we're taking for the pipeline modes seem to be rather right the way you have implemented them. The docs, comments and tests needed quite a few tweaks and edits to be more consistent. There were some grammar mistakes, some frenchisms. I'm hoping that there won't be any issues, but let's be honest, I am definitely sure there will be some more tuning required. It comes down to if we want this set of features, and I do to be able to expand tests in core with the extended query protocol and pipelines, knowing that there is an ask for out-of-core projects. This one being reachable with a COPY gave me a huge smile: +message type 0x5a arrived from server while idle So let's take one step here, I have applied the main patch. I am really excited by the possibilities all this stuff offers. Attached are the remaining pieces, split here because they are different bullet points: - Tests for implicit transactions with various commands, with some edits. - Prompt support, with more edits. I'm putting these on standby for a few days, to let the buildfarm digest the main change. -- Michael
From 035f19d120b181094b00dee21acb0b4ce4cfc1e1 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Fri, 21 Feb 2025 11:27:36 +0900
Subject: [PATCH v10 1/2] Add tests with implicit transaction blocks and
pipelines
This checks interactions with $subject for the following commands that
have dedicated behaviors in transaction blocks:
- SET LOCAL
- REINDEX CONCURRENTLY
- VACUUM
- Subtransactions
---
src/test/regress/expected/psql_pipeline.out | 112 ++++++++++++++++++++
src/test/regress/sql/psql_pipeline.sql | 72 +++++++++++++
2 files changed, 184 insertions(+)
diff --git src/test/regress/expected/psql_pipeline.out src/test/regress/expected/psql_pipeline.out
index bdf5a99d09b0..f4603d2b66ae 100644
--- src/test/regress/expected/psql_pipeline.out
+++ src/test/regress/expected/psql_pipeline.out
@@ -585,5 +585,117 @@ PQsendQuery not allowed in pipeline mode
1
(1 row)
+--
+-- Pipelines and transaction blocks
+--
+-- SET LOCAL will issue a warning when modifying a GUC outside of a
+-- transaction block. The change will still be valid as a pipeline
+-- runs within an implicit transaction block. Sending a sync will
+-- commit the implicit transaction block. The first command after a
+-- sync will not be seen as belonging to a pipeline.
+\startpipeline
+SET LOCAL statement_timeout='1h' \bind \g
+SHOW statement_timeout \bind \g
+\syncpipeline
+SHOW statement_timeout \bind \g
+SET LOCAL statement_timeout='2h' \bind \g
+SHOW statement_timeout \bind \g
+\endpipeline
+WARNING: SET LOCAL can only be used in transaction blocks
+ statement_timeout
+-------------------
+ 1h
+(1 row)
+
+ statement_timeout
+-------------------
+ 0
+(1 row)
+
+ statement_timeout
+-------------------
+ 2h
+(1 row)
+
+-- REINDEX CONCURRENTLY fails if not the first command in a pipeline.
+\startpipeline
+SELECT $1 \bind 1 \g
+REINDEX TABLE CONCURRENTLY psql_pipeline \bind \g
+SELECT $1 \bind 2 \g
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+ERROR: REINDEX CONCURRENTLY cannot run inside a transaction block
+-- REINDEX CONCURRENTLY works if it is the first command in a pipeline.
+\startpipeline
+REINDEX TABLE CONCURRENTLY psql_pipeline \bind \g
+SELECT $1 \bind 2 \g
+\endpipeline
+ ?column?
+----------
+ 2
+(1 row)
+
+-- Subtransactions are not allowed in a pipeline.
+\startpipeline
+SAVEPOINT a \bind \g
+SELECT $1 \bind 1 \g
+ROLLBACK TO SAVEPOINT a \bind \g
+SELECT $1 \bind 2 \g
+\endpipeline
+ERROR: SAVEPOINT can only be used in transaction blocks
+-- LOCK fails as the first command in a pipeline, as not seen in an
+-- implicit transaction block.
+\startpipeline
+LOCK psql_pipeline \bind \g
+SELECT $1 \bind 2 \g
+\endpipeline
+ERROR: LOCK TABLE can only be used in transaction blocks
+-- LOCK succeeds as it is not the first command in a pipeline,
+-- seen in an implicit transaction block.
+\startpipeline
+SELECT $1 \bind 1 \g
+LOCK psql_pipeline \bind \g
+SELECT $1 \bind 2 \g
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+ ?column?
+----------
+ 2
+(1 row)
+
+-- VACUUM works as the first command in a pipeline.
+\startpipeline
+VACUUM psql_pipeline \bind \g
+\endpipeline
+-- VACUUM fails when not the first command in a pipeline.
+\startpipeline
+SELECT 1 \bind \g
+VACUUM psql_pipeline \bind \g
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+ERROR: VACUUM cannot run inside a transaction block
+-- VACUUM works after a \syncpipeline.
+\startpipeline
+SELECT 1 \bind \g
+\syncpipeline
+VACUUM psql_pipeline \bind \g
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
-- Clean up
DROP TABLE psql_pipeline;
diff --git src/test/regress/sql/psql_pipeline.sql src/test/regress/sql/psql_pipeline.sql
index 38e48054eeb9..ec62e6c5f247 100644
--- src/test/regress/sql/psql_pipeline.sql
+++ src/test/regress/sql/psql_pipeline.sql
@@ -350,5 +350,77 @@ SELECT 1;
SELECT 1;
\endpipeline
+--
+-- Pipelines and transaction blocks
+--
+
+-- SET LOCAL will issue a warning when modifying a GUC outside of a
+-- transaction block. The change will still be valid as a pipeline
+-- runs within an implicit transaction block. Sending a sync will
+-- commit the implicit transaction block. The first command after a
+-- sync will not be seen as belonging to a pipeline.
+\startpipeline
+SET LOCAL statement_timeout='1h' \bind \g
+SHOW statement_timeout \bind \g
+\syncpipeline
+SHOW statement_timeout \bind \g
+SET LOCAL statement_timeout='2h' \bind \g
+SHOW statement_timeout \bind \g
+\endpipeline
+
+-- REINDEX CONCURRENTLY fails if not the first command in a pipeline.
+\startpipeline
+SELECT $1 \bind 1 \g
+REINDEX TABLE CONCURRENTLY psql_pipeline \bind \g
+SELECT $1 \bind 2 \g
+\endpipeline
+
+-- REINDEX CONCURRENTLY works if it is the first command in a pipeline.
+\startpipeline
+REINDEX TABLE CONCURRENTLY psql_pipeline \bind \g
+SELECT $1 \bind 2 \g
+\endpipeline
+
+-- Subtransactions are not allowed in a pipeline.
+\startpipeline
+SAVEPOINT a \bind \g
+SELECT $1 \bind 1 \g
+ROLLBACK TO SAVEPOINT a \bind \g
+SELECT $1 \bind 2 \g
+\endpipeline
+
+-- LOCK fails as the first command in a pipeline, as not seen in an
+-- implicit transaction block.
+\startpipeline
+LOCK psql_pipeline \bind \g
+SELECT $1 \bind 2 \g
+\endpipeline
+
+-- LOCK succeeds as it is not the first command in a pipeline,
+-- seen in an implicit transaction block.
+\startpipeline
+SELECT $1 \bind 1 \g
+LOCK psql_pipeline \bind \g
+SELECT $1 \bind 2 \g
+\endpipeline
+
+-- VACUUM works as the first command in a pipeline.
+\startpipeline
+VACUUM psql_pipeline \bind \g
+\endpipeline
+
+-- VACUUM fails when not the first command in a pipeline.
+\startpipeline
+SELECT 1 \bind \g
+VACUUM psql_pipeline \bind \g
+\endpipeline
+
+-- VACUUM works after a \syncpipeline.
+\startpipeline
+SELECT 1 \bind \g
+\syncpipeline
+VACUUM psql_pipeline \bind \g
+\endpipeline
+
-- Clean up
DROP TABLE psql_pipeline;
--
2.47.2
From b183638bb31c75fab28ecf67112304b52bb12dce Mon Sep 17 00:00:00 2001
From: Anthonin Bonnefoy <anthonin.bonne...@datadoghq.com>
Date: Tue, 18 Feb 2025 15:32:31 +0100
Subject: [PATCH v10 2/2] Add prompt interpolation and variables for psql
pipeline
Add %P prompt interpolation that reports the status pipeline: on, off or
abort. Additionally, 3 new special variables are added to report the
status of an ongoing pipeline:
- PIPELINE_SYNC_COUNT: reports the number of piped sync
- PIPELINE_COMMAND_COUNT: reports the number of piped commands, a
command being either \bind, \bind_named, \close or \parse
- PIPELINE_RESULT_COUNT: reports the results available to read with
\getresults
---
src/bin/psql/common.c | 27 +++++++++++++++++-
src/bin/psql/prompt.c | 14 ++++++++++
src/bin/psql/startup.c | 5 ++++
doc/src/sgml/ref/psql-ref.sgml | 50 ++++++++++++++++++++++++++++++++++
4 files changed, 95 insertions(+), 1 deletion(-)
diff --git src/bin/psql/common.c src/bin/psql/common.c
index bc8c40898f71..ed340a466f99 100644
--- src/bin/psql/common.c
+++ src/bin/psql/common.c
@@ -524,6 +524,26 @@ SetShellResultVariables(int wait_result)
}
+/*
+ * Set special pipeline variables
+ * - PIPELINE_SYNC_COUNT: The number of piped syncs
+ * - PIPELINE_COMMAND_COUNT: The number of piped commands
+ * - PIPELINE_RESULT_COUNT: The number of results available to read
+ */
+static void
+SetPipelineVariables(void)
+{
+ char buf[32];
+
+ snprintf(buf, sizeof(buf), "%d", pset.piped_syncs);
+ SetVariable(pset.vars, "PIPELINE_SYNC_COUNT", buf);
+ snprintf(buf, sizeof(buf), "%d", pset.piped_commands);
+ SetVariable(pset.vars, "PIPELINE_COMMAND_COUNT", buf);
+ snprintf(buf, sizeof(buf), "%d", pset.available_results);
+ SetVariable(pset.vars, "PIPELINE_RESULT_COUNT", buf);
+}
+
+
/*
* ClearOrSaveResult
*
@@ -1661,6 +1681,8 @@ ExecQueryAndProcessResults(const char *query,
CheckConnection();
+ SetPipelineVariables();
+
return -1;
}
@@ -1669,8 +1691,10 @@ ExecQueryAndProcessResults(const char *query,
{
/*
* We are in a pipeline and have not reached the pipeline end, or
- * there was no request to read pipeline results, exit.
+ * there was no request to read pipeline results. Update the psql
+ * variables tracking the pipeline activity and exit.
*/
+ SetPipelineVariables();
return 1;
}
@@ -2105,6 +2129,7 @@ ExecQueryAndProcessResults(const char *query,
Assert(pset.available_results == 0);
}
Assert(pset.requested_results == 0);
+ SetPipelineVariables();
/* may need this to recover from conn loss during COPY */
if (!CheckConnection())
diff --git src/bin/psql/prompt.c src/bin/psql/prompt.c
index 08a14feb3c3f..3aa7d2d06c80 100644
--- src/bin/psql/prompt.c
+++ src/bin/psql/prompt.c
@@ -31,6 +31,7 @@
* sockets, "[local:/dir/name]" if not default
* %m - like %M, but hostname only (before first dot), or always "[local]"
* %p - backend pid
+ * %P - pipeline status: on, off or abort
* %> - database server port number
* %n - database user name
* %s - service
@@ -181,6 +182,19 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
snprintf(buf, sizeof(buf), "%d", pid);
}
break;
+ /* pipeline status */
+ case 'P':
+ {
+ PGpipelineStatus status = PQpipelineStatus(pset.db);
+
+ if (status == PQ_PIPELINE_ON)
+ strlcpy(buf, "on", sizeof(buf));
+ else if (status == PQ_PIPELINE_ABORTED)
+ strlcpy(buf, "abort", sizeof(buf));
+ else
+ strlcpy(buf, "off", sizeof(buf));
+ break;
+ }
case '0':
case '1':
diff --git src/bin/psql/startup.c src/bin/psql/startup.c
index 703f3f582c17..5018eedf1e57 100644
--- src/bin/psql/startup.c
+++ src/bin/psql/startup.c
@@ -205,6 +205,11 @@ main(int argc, char *argv[])
SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3);
SetVariableBool(pset.vars, "SHOW_ALL_RESULTS");
+ /* Initialize pipeline variables */
+ SetVariable(pset.vars, "PIPELINE_SYNC_COUNT", "0");
+ SetVariable(pset.vars, "PIPELINE_COMMAND_COUNT", "0");
+ SetVariable(pset.vars, "PIPELINE_RESULT_COUNT", "0");
+
parse_psql_options(argc, argv, &options);
/*
diff --git doc/src/sgml/ref/psql-ref.sgml doc/src/sgml/ref/psql-ref.sgml
index a8a0b694bbf0..32130242e9c8 100644
--- doc/src/sgml/ref/psql-ref.sgml
+++ doc/src/sgml/ref/psql-ref.sgml
@@ -3727,6 +3727,12 @@ testdb=> <userinput>\setenv LESS -imx4F</userinput>
generate one result to get.
</para>
+ <para>
+ When pipeline mode is active, a dedicated prompt variable is available
+ to report the pipeline status.
+ See <xref linkend="app-psql-prompting-p-uc"/> for more details
+ </para>
+
<para>
Example:
<programlisting>
@@ -4501,6 +4507,39 @@ bar
</listitem>
</varlistentry>
+ <varlistentry id="app-psql-variables-pipeline-command-count">
+ <term><varname>PIPELINE_COMMAND_COUNT</varname></term>
+ <listitem>
+ <para>
+ The number of commands generated by <literal>\bind</literal>,
+ <literal>\bind_named</literal>, <literal>\close</literal> or
+ <literal>\parse</literal> queued in an ongoing pipeline.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="app-psql-variables-pipeline-result-count">
+ <term><varname>PIPELINE_RESULT_COUNT</varname></term>
+ <listitem>
+ <para>
+ The number of commands of an ongoing pipeline that were followed
+ by either a <command>\flushrequest</command> or a
+ <command>\syncpipeline</command>, forcing the server to send the
+ results. These results can be retrieved with
+ <command>\getresults</command>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="app-psql-variables-pipeline-sync-count">
+ <term><varname>PIPELINE_SYNC_COUNT</varname></term>
+ <listitem>
+ <para>
+ The number of sync messages queued in an ongoing pipeline.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-variables-port">
<term><varname>PORT</varname></term>
<listitem>
@@ -4900,6 +4939,17 @@ testdb=> <userinput>INSERT INTO my_table VALUES (:'content');</userinput>
</listitem>
</varlistentry>
+ <varlistentry id="app-psql-prompting-p-uc">
+ <term><literal>%P</literal></term>
+ <listitem>
+ <para>
+ Pipeline status: <literal>off</literal> when not in a pipeline,
+ <literal>on</literal> when in an ongoing pipeline or
+ <literal>abort</literal> when in an aborted pipeline.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-prompting-r">
<term><literal>%R</literal></term>
<listitem>
--
2.47.2
signature.asc
Description: PGP signature
