public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: [email protected]
To: [email protected]
Subject: Re: BUG #19494: Error on transaction commit inside pipeline triggers psql's Assert
Date: Thu, 28 May 2026 14:26:06 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>

On Thu, May 28, 2026 at 12:51:38PM +0900, Michael Paquier wrote:
> I am completely sure yet, but it looks like we will need to be smarter
> with the handling of the number of piped commands by tracking them
> across the syncs in the shape of a queue, or something like that?  So
> it feels like we need to think harder about the tracking of this
> activity depending on the state of the pipeline we're in.  Or we could
> lift some of these assertions, but that would not be right to me.

Hmm.  Taking a step back this would be overcomplicating things.  As
long as we are careful to consume the synced results still in a
pipeline, it looks like we should be fine.  While digging into it, I
have found a third assertion that was triggerable with
available_results at the end of the pipeline, once I began mixing
\getresults with a deferred error.

This stuff is tricky enough that I may not have overseen all the
patterns possible, of course, at least this is progress.

Alexander, what do you think?
--
Michael

From f9ce93fc1e9bb47f4a50605a2b836bb9cc278989 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Thu, 28 May 2026 14:20:47 +0900
Subject: [PATCH] psql: Fix failures with deferred errors in pipelines

---
 src/bin/psql/common.c                       | 48 ++++++++---
 src/test/regress/expected/psql_pipeline.out | 91 +++++++++++++++++++++
 src/test/regress/sql/psql_pipeline.sql      | 39 +++++++++
 3 files changed, 166 insertions(+), 12 deletions(-)

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 1a4e2ea0da82..3698f37c742c 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1499,11 +1499,19 @@ discardAbortedPipelineResults(void)
 		}
 		else if (res == NULL)
 		{
-			/* A query was processed, decrement the counters */
-			Assert(pset.available_results > 0);
-			Assert(pset.requested_results > 0);
-			pset.available_results--;
-			pset.requested_results--;
+			/*
+			 * A query was processed, decrement the counters.
+			 *
+			 * It is possible to get here with available_results == 0 when an
+			 * error is generated by the Sync message processing itself.
+			 * Such errors are not counted in available_results because they
+			 * are not associated with a piped command.  In that case, skip
+			 * the counter decrements and continue to find the sync result.
+			 */
+			if (pset.available_results > 0)
+				pset.available_results--;
+			if (pset.requested_results > 0)
+				pset.requested_results--;
 		}
 
 		if (pset.requested_results == 0)
@@ -2175,14 +2183,30 @@ ExecQueryAndProcessResults(const char *query,
 
 	if (end_pipeline)
 	{
-		/* after a pipeline is processed, pipeline piped_syncs should be 0 */
-		Assert(pset.piped_syncs == 0);
-		/* all commands have been processed */
-		Assert(pset.piped_commands == 0);
-		/* all results were read */
-		Assert(pset.available_results == 0);
+		/*
+		 * Reset available/requested results.  Normally these are already 0,
+		 * but an error generated by Sync processing itself can leave some of
+		 * them behind.  Consume them before exiting pipeline mode.
+		 */
+		while (pset.piped_syncs > 0)
+		{
+			PGresult   *remaining = PQgetResult(pset.db);
+
+			if (remaining == NULL)
+				continue;
+			if (PQresultStatus(remaining) == PGRES_PIPELINE_SYNC)
+				pset.piped_syncs--;
+			PQclear(remaining);
+		}
+		pset.piped_syncs = 0;
+		pset.piped_commands = 0;
+		pset.available_results = 0;
+		pset.requested_results = 0;
+
+		if (PQpipelineStatus(pset.db) != PQ_PIPELINE_OFF)
+			PQexitPipelineMode(pset.db);
 	}
-	Assert(pset.requested_results == 0);
+
 	SetPipelineVariables();
 
 	/* may need this to recover from conn loss during COPY */
diff --git a/src/test/regress/expected/psql_pipeline.out b/src/test/regress/expected/psql_pipeline.out
index a0816fb10b68..66e1e4f2ef5e 100644
--- a/src/test/regress/expected/psql_pipeline.out
+++ b/src/test/regress/expected/psql_pipeline.out
@@ -764,5 +764,96 @@ VACUUM psql_pipeline \bind \sendpipeline
         1
 (1 row)
 
+-- Deferred constraint violation at commit time in a pipeline.
+CREATE TABLE psql_pipeline_defer (a INTEGER PRIMARY KEY DEFERRABLE INITIALLY DEFERRED);
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) RETURNING * \bind 1 \sendpipeline
+\endpipeline
+ a 
+---
+ 1
+ 1
+(2 rows)
+
+ERROR:  duplicate key value violates unique constraint "psql_pipeline_defer_pkey"
+DETAIL:  Key (a)=(1) already exists.
+-- Same with \syncpipeline and commands after the failing sync.
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\endpipeline
+ERROR:  duplicate key value violates unique constraint "psql_pipeline_defer_pkey"
+DETAIL:  Key (a)=(1) already exists.
+   ?column?   
+--------------
+ after_sync_1
+(1 row)
+
+-- More patterns with more \syncpipeline, more commands and \getresults
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\getresults
+ERROR:  duplicate key value violates unique constraint "psql_pipeline_defer_pkey"
+DETAIL:  Key (a)=(1) already exists.
+SELECT $1 \bind 'after_sync_2' \sendpipeline
+\endpipeline
+   ?column?   
+--------------
+ after_sync_1
+(1 row)
+
+   ?column?   
+--------------
+ after_sync_2
+(1 row)
+
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+\getresults
+ERROR:  duplicate key value violates unique constraint "psql_pipeline_defer_pkey"
+DETAIL:  Key (a)=(1) already exists.
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\getresults
+SELECT $1 \bind 'after_sync_2' \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+SELECT $1 \bind 'after_sync_3' \sendpipeline
+SELECT $1 \bind 'after_sync_4' \sendpipeline
+SELECT $1 \bind 'after_sync_5' \sendpipeline
+\endpipeline
+   ?column?   
+--------------
+ after_sync_1
+(1 row)
+
+   ?column?   
+--------------
+ after_sync_2
+(1 row)
+
+   ?column?   
+--------------
+ after_sync_3
+(1 row)
+
+   ?column?   
+--------------
+ after_sync_4
+(1 row)
+
+   ?column?   
+--------------
+ after_sync_5
+(1 row)
+
+ERROR:  duplicate key value violates unique constraint "psql_pipeline_defer_pkey"
+DETAIL:  Key (a)=(1) already exists.
+DROP TABLE psql_pipeline_defer;
 -- Clean up
 DROP TABLE psql_pipeline;
diff --git a/src/test/regress/sql/psql_pipeline.sql b/src/test/regress/sql/psql_pipeline.sql
index 6788dceee2e9..ec68e48e25aa 100644
--- a/src/test/regress/sql/psql_pipeline.sql
+++ b/src/test/regress/sql/psql_pipeline.sql
@@ -438,5 +438,44 @@ SELECT 1 \bind \sendpipeline
 VACUUM psql_pipeline \bind \sendpipeline
 \endpipeline
 
+-- Deferred constraint violation at commit time in a pipeline.
+CREATE TABLE psql_pipeline_defer (a INTEGER PRIMARY KEY DEFERRABLE INITIALLY DEFERRED);
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) RETURNING * \bind 1 \sendpipeline
+\endpipeline
+
+-- Same with \syncpipeline and commands after the failing sync.
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\endpipeline
+
+-- More patterns with more \syncpipeline, more commands and \getresults
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\getresults
+SELECT $1 \bind 'after_sync_2' \sendpipeline
+\endpipeline
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+\getresults
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\getresults
+SELECT $1 \bind 'after_sync_2' \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+SELECT $1 \bind 'after_sync_3' \sendpipeline
+SELECT $1 \bind 'after_sync_4' \sendpipeline
+SELECT $1 \bind 'after_sync_5' \sendpipeline
+\endpipeline
+
+DROP TABLE psql_pipeline_defer;
+
 -- Clean up
 DROP TABLE psql_pipeline;
-- 
2.54.0



Attachments:

  [text/plain] 0001-psql-Fix-failures-with-deferred-errors-in-pipelines.patch (7.3K, 2-0001-psql-Fix-failures-with-deferred-errors-in-pipelines.patch)
  download | inline diff:
From f9ce93fc1e9bb47f4a50605a2b836bb9cc278989 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Thu, 28 May 2026 14:20:47 +0900
Subject: [PATCH] psql: Fix failures with deferred errors in pipelines

---
 src/bin/psql/common.c                       | 48 ++++++++---
 src/test/regress/expected/psql_pipeline.out | 91 +++++++++++++++++++++
 src/test/regress/sql/psql_pipeline.sql      | 39 +++++++++
 3 files changed, 166 insertions(+), 12 deletions(-)

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 1a4e2ea0da82..3698f37c742c 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1499,11 +1499,19 @@ discardAbortedPipelineResults(void)
 		}
 		else if (res == NULL)
 		{
-			/* A query was processed, decrement the counters */
-			Assert(pset.available_results > 0);
-			Assert(pset.requested_results > 0);
-			pset.available_results--;
-			pset.requested_results--;
+			/*
+			 * A query was processed, decrement the counters.
+			 *
+			 * It is possible to get here with available_results == 0 when an
+			 * error is generated by the Sync message processing itself.
+			 * Such errors are not counted in available_results because they
+			 * are not associated with a piped command.  In that case, skip
+			 * the counter decrements and continue to find the sync result.
+			 */
+			if (pset.available_results > 0)
+				pset.available_results--;
+			if (pset.requested_results > 0)
+				pset.requested_results--;
 		}
 
 		if (pset.requested_results == 0)
@@ -2175,14 +2183,30 @@ ExecQueryAndProcessResults(const char *query,
 
 	if (end_pipeline)
 	{
-		/* after a pipeline is processed, pipeline piped_syncs should be 0 */
-		Assert(pset.piped_syncs == 0);
-		/* all commands have been processed */
-		Assert(pset.piped_commands == 0);
-		/* all results were read */
-		Assert(pset.available_results == 0);
+		/*
+		 * Reset available/requested results.  Normally these are already 0,
+		 * but an error generated by Sync processing itself can leave some of
+		 * them behind.  Consume them before exiting pipeline mode.
+		 */
+		while (pset.piped_syncs > 0)
+		{
+			PGresult   *remaining = PQgetResult(pset.db);
+
+			if (remaining == NULL)
+				continue;
+			if (PQresultStatus(remaining) == PGRES_PIPELINE_SYNC)
+				pset.piped_syncs--;
+			PQclear(remaining);
+		}
+		pset.piped_syncs = 0;
+		pset.piped_commands = 0;
+		pset.available_results = 0;
+		pset.requested_results = 0;
+
+		if (PQpipelineStatus(pset.db) != PQ_PIPELINE_OFF)
+			PQexitPipelineMode(pset.db);
 	}
-	Assert(pset.requested_results == 0);
+
 	SetPipelineVariables();
 
 	/* may need this to recover from conn loss during COPY */
diff --git a/src/test/regress/expected/psql_pipeline.out b/src/test/regress/expected/psql_pipeline.out
index a0816fb10b68..66e1e4f2ef5e 100644
--- a/src/test/regress/expected/psql_pipeline.out
+++ b/src/test/regress/expected/psql_pipeline.out
@@ -764,5 +764,96 @@ VACUUM psql_pipeline \bind \sendpipeline
         1
 (1 row)
 
+-- Deferred constraint violation at commit time in a pipeline.
+CREATE TABLE psql_pipeline_defer (a INTEGER PRIMARY KEY DEFERRABLE INITIALLY DEFERRED);
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) RETURNING * \bind 1 \sendpipeline
+\endpipeline
+ a 
+---
+ 1
+ 1
+(2 rows)
+
+ERROR:  duplicate key value violates unique constraint "psql_pipeline_defer_pkey"
+DETAIL:  Key (a)=(1) already exists.
+-- Same with \syncpipeline and commands after the failing sync.
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\endpipeline
+ERROR:  duplicate key value violates unique constraint "psql_pipeline_defer_pkey"
+DETAIL:  Key (a)=(1) already exists.
+   ?column?   
+--------------
+ after_sync_1
+(1 row)
+
+-- More patterns with more \syncpipeline, more commands and \getresults
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\getresults
+ERROR:  duplicate key value violates unique constraint "psql_pipeline_defer_pkey"
+DETAIL:  Key (a)=(1) already exists.
+SELECT $1 \bind 'after_sync_2' \sendpipeline
+\endpipeline
+   ?column?   
+--------------
+ after_sync_1
+(1 row)
+
+   ?column?   
+--------------
+ after_sync_2
+(1 row)
+
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+\getresults
+ERROR:  duplicate key value violates unique constraint "psql_pipeline_defer_pkey"
+DETAIL:  Key (a)=(1) already exists.
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\getresults
+SELECT $1 \bind 'after_sync_2' \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+SELECT $1 \bind 'after_sync_3' \sendpipeline
+SELECT $1 \bind 'after_sync_4' \sendpipeline
+SELECT $1 \bind 'after_sync_5' \sendpipeline
+\endpipeline
+   ?column?   
+--------------
+ after_sync_1
+(1 row)
+
+   ?column?   
+--------------
+ after_sync_2
+(1 row)
+
+   ?column?   
+--------------
+ after_sync_3
+(1 row)
+
+   ?column?   
+--------------
+ after_sync_4
+(1 row)
+
+   ?column?   
+--------------
+ after_sync_5
+(1 row)
+
+ERROR:  duplicate key value violates unique constraint "psql_pipeline_defer_pkey"
+DETAIL:  Key (a)=(1) already exists.
+DROP TABLE psql_pipeline_defer;
 -- Clean up
 DROP TABLE psql_pipeline;
diff --git a/src/test/regress/sql/psql_pipeline.sql b/src/test/regress/sql/psql_pipeline.sql
index 6788dceee2e9..ec68e48e25aa 100644
--- a/src/test/regress/sql/psql_pipeline.sql
+++ b/src/test/regress/sql/psql_pipeline.sql
@@ -438,5 +438,44 @@ SELECT 1 \bind \sendpipeline
 VACUUM psql_pipeline \bind \sendpipeline
 \endpipeline
 
+-- Deferred constraint violation at commit time in a pipeline.
+CREATE TABLE psql_pipeline_defer (a INTEGER PRIMARY KEY DEFERRABLE INITIALLY DEFERRED);
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) RETURNING * \bind 1 \sendpipeline
+\endpipeline
+
+-- Same with \syncpipeline and commands after the failing sync.
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\endpipeline
+
+-- More patterns with more \syncpipeline, more commands and \getresults
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\getresults
+SELECT $1 \bind 'after_sync_2' \sendpipeline
+\endpipeline
+\startpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+\syncpipeline
+\getresults
+SELECT $1 \bind 'after_sync_1' \sendpipeline
+\getresults
+SELECT $1 \bind 'after_sync_2' \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+INSERT INTO psql_pipeline_defer VALUES ($1), ($1) \bind 1 \sendpipeline
+SELECT $1 \bind 'after_sync_3' \sendpipeline
+SELECT $1 \bind 'after_sync_4' \sendpipeline
+SELECT $1 \bind 'after_sync_5' \sendpipeline
+\endpipeline
+
+DROP TABLE psql_pipeline_defer;
+
 -- Clean up
 DROP TABLE psql_pipeline;
-- 
2.54.0



  [application/pgp-signature] signature.asc (833B, 3-signature.asc)
  download

view thread (10+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: BUG #19494: Error on transaction commit inside pipeline triggers psql's Assert
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox