On Fri, 26 Sep 2025 11:44:42 +0900
Yugo Nagata <[email protected]> wrote:

> On Thu, 25 Sep 2025 17:17:36 +0800
> Chao Li <[email protected]> wrote:
> 
> > Hi Yugo,
> > 
> > Thanks for the patch. After reviewing it, I got a few small comments:
> 
> Thank you for your reviewing and comments.
> 
> > > On Sep 25, 2025, at 15:22, Yugo Nagata <[email protected]> wrote:
> > > 
> > > -- 
> > > Yugo Nagata <[email protected] <mailto:[email protected]>>
> > > <v13-0003-Improve-error-messages-for-errors-that-cause-cli.patch><v13-0002-Add-continue-on-error-option.patch><v13-0001-Fix-assertion-failure-and-verbose-messages-in-pi.patch>
> > 
> > 
> > 1 - 0001
> > ```
> > @@ -3265,6 +3271,7 @@ readCommandResponse(CState *st, MetaCommand meta, 
> > char *varprefix)
> >     PGresult   *res;
> >     PGresult   *next_res;
> >     int                     qrynum = 0;
> > +   char       *errmsg;
> > ```
> > 
> > I think we should initialize errmsg to NULL. Compiler won’t auto initialize 
> > a local variable. If it happens to not enter the while loop, then errmsg 
> > will hold a random value, then pg_free(errmsg) will have trouble.
> 
> I think this initialization is unnecessary, just like for res and next_res.
> If the code happens not to enter the while loop, pg_free(errmsg) will not be
> called anyway, since the error: label is only reachable from inside the loop.
> 
> > 2 - 0002
> > ```
> > +       <para>
> > +        Allows clients to continue their run even if an SQL statement 
> > fails due to
> > +        errors other than serialization or deadlock. Unlike serialization 
> > and deadlock
> > +        failures, clients do not retry the same transactions but start new 
> > transaction.
> > +        This option is useful when your custom script may raise errors due 
> > to some
> > +        reason like unique constraints violation. Without this option, the 
> > client is
> > +        aborted after such errors.
> > +       </para>
> > ```
> > 
> > A few nit suggestions:
> > 
> > * “continue their run” => “continue running”
> 
> Fixed.
> 
> > * “clients to not retry the same transactions but start new transaction” => 
> > “clients do not retry the same transaction but start a new transaction 
> > instead"
> 
> I see your point. Maybe we could follow Anthonin Bonnefoy's suggestion
> to use "proceed to the next transaction", as it may sound a bit more natural.
> 
> > * “due to some reason like” => “for reasons such as"
> 
> Fixed.
> 
> > 3 - 0002
> > ```
> > +    * Without --continue-on-error:
> >      * failed (the number of failed transactions) =
> > ```
> > 
> > Maybe add an empty line after “without” line.
> 
> Makes sense. Fixed.
>  
> > 4 - 0002
> > ```
> > +    * When --continue-on-error is specified:
> > +    *
> > +    * failed (number of failed transactions) =
> > ```
> > 
> > Maybe change to “With ---continue-on-error”, which sounds consistent with 
> > the previous “without”.
> 
> Fixed.
> 
> > 5 - 0002
> > ```
> > +   int64           other_sql_failures; /* number of failed transactions for
> > +                                                                    * 
> > reasons other than
> > +                                                                    * 
> > serialization/deadlock failure, which
> > +                                                                    * is 
> > counted if --continue-on-error is
> > +                                                                    * 
> > specified */
> > ```
> > 
> > How about rename this variable to “sql_errors”, which reflects to the new 
> > option name.
> 
> I think it’s better to keep the current name, since the variable counts 
> failed transactions,
> even though that happens to be equivalent to the number of SQL errors. It’s 
> also consistent
> with the other variables, serialization_failures and deadlock_failures.
> 
> > 6 - 0002
> > ```
> > @@ -4571,6 +4594,8 @@ getResultString(bool skipped, EStatus estatus)
> >                             return "serialization";
> >                     case ESTATUS_DEADLOCK_ERROR:
> >                             return "deadlock";
> > +                   case ESTATUS_OTHER_SQL_ERROR:
> > +                           return "other”;
> > ```
> > 
> > I think this can just return “error”. I checked where this function is 
> > called, there will not be other words such as “error” appended.
> 
> getResultString() is called to get a string that represents the type of error
> causing the transaction failure, so simply returning "error" doesn’t seem very
> useful.
>  
> > 7 - 0002
> > ```
> >     /* it can be non-zero only if max_tries is not equal to one */
> > @@ -6569,6 +6602,10 @@ printResults(StatsData *total,
> >                                                        
> > sstats->deadlock_failures,
> >                                                        (100.0 * 
> > sstats->deadlock_failures /
> >                                                             
> > script_total_cnt));
> > +                                           printf(" - number of other 
> > failures: " INT64_FORMAT " (%.3f%%)\n",
> > +                                                      
> > sstats->other_sql_failures,
> > +                                                      (100.0 * 
> > sstats->other_sql_failures /
> > +                                                           
> > script_total_cnt));
> > ```
> > 
> > Do we only want to print this number when “―continue-on-error” is given?
> 
> We could do that, but this message is printed only when
> --failures-detailed is specified. So I think users would not mind
> if it shows that the number of other failures is zero, even when
> --continue-on-error is not specified.
> 
> I would appreciate hearing other people's opinions on this.
> 
> 
> I've attached updated patches that include fixes for some of your
> suggestions and for Anthonin Bonnefoy's suggestion on the documentation.
> 
> I also split the patch according to Fujii-san's suggestion.

Fujii-san, thank you for committing the patch that fixes the assertion failure.
I've attached the remaining patches so that cfbot stays green.

Regards,
Yugo Nagata



-- 
Yugo Nagata <[email protected]>
>From 353376bd49fa322c222049fa2fada540b0b7f2b3 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <[email protected]>
Date: Thu, 10 Jul 2025 17:21:05 +0900
Subject: [PATCH v15 3/3] pgbench: Improve error messages for errors that cause
 client abortion

This commit modifies relevant error messages to explicitly indicate that the
client was aborted. As part of this change, pg_log_error was replaced with
commandFailed().
---
 src/bin/pgbench/pgbench.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 18bce17a245..9afdf9e6d6c 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3321,8 +3321,7 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 			case PGRES_EMPTY_QUERY: /* may be used for testing no-op overhead */
 				if (is_last && meta == META_GSET)
 				{
-					pg_log_error("client %d script %d command %d query %d: expected one row, got %d",
-								 st->id, st->use_file, st->command, qrynum, 0);
+					commandFailed(st, "gset", psprintf("expected one row, got %d", 0));
 					st->estatus = ESTATUS_META_COMMAND_ERROR;
 					goto error;
 				}
@@ -3336,8 +3335,7 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 					if (meta == META_GSET && ntuples != 1)
 					{
 						/* under \gset, report the error */
-						pg_log_error("client %d script %d command %d query %d: expected one row, got %d",
-									 st->id, st->use_file, st->command, qrynum, PQntuples(res));
+						commandFailed(st, "gset", psprintf("expected one row, got %d", PQntuples(res)));
 						st->estatus = ESTATUS_META_COMMAND_ERROR;
 						goto error;
 					}
@@ -3351,18 +3349,18 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 					for (int fld = 0; fld < PQnfields(res); fld++)
 					{
 						char	   *varname = PQfname(res, fld);
+						char	   *cmd = (meta == META_ASET ? "aset" : "gset");
 
 						/* allocate varname only if necessary, freed below */
 						if (*varprefix != '\0')
 							varname = psprintf("%s%s", varprefix, varname);
 
 						/* store last row result as a string */
-						if (!putVariable(&st->variables, meta == META_ASET ? "aset" : "gset", varname,
+						if (!putVariable(&st->variables, cmd, varname,
 										 PQgetvalue(res, ntuples - 1, fld)))
 						{
 							/* internal error */
-							pg_log_error("client %d script %d command %d query %d: error storing into variable %s",
-										 st->id, st->use_file, st->command, qrynum, varname);
+							commandFailed(st, cmd, psprintf("error storing into variable %s", varname));
 							st->estatus = ESTATUS_META_COMMAND_ERROR;
 							goto error;
 						}
@@ -3397,8 +3395,7 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 
 			default:
 				/* anything else is unexpected */
-				pg_log_error("client %d script %d aborted in command %d query %d: %s",
-							 st->id, st->use_file, st->command, qrynum, errmsg);
+				commandFailed(st, "SQL", errmsg);
 				goto error;
 		}
 
-- 
2.43.0

>From 426e6cb4d711f61a792c3d4ec38e2a07bd59d2ac Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 19 Sep 2025 16:54:49 +0900
Subject: [PATCH v15 2/3] pgbench: Add --continue-on-error option

When the option is set, client rolls back the failed transaction and starts a
new one when its transaction fails due to the reason other than the deadlock and
serialization failure.
---
 doc/src/sgml/ref/pgbench.sgml                | 64 ++++++++++++++++----
 src/bin/pgbench/pgbench.c                    | 60 +++++++++++++++---
 src/bin/pgbench/t/001_pgbench_with_server.pl | 22 +++++++
 3 files changed, 127 insertions(+), 19 deletions(-)

diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index a5edf612443..0305f4553d3 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -76,9 +76,8 @@ tps = 896.967014 (without initial connection time)
   and number of transactions per client); these will be equal unless the run
   failed before completion or some SQL command(s) failed.  (In
   <option>-T</option> mode, only the actual number of transactions is printed.)
-  The next line reports the number of failed transactions due to
-  serialization or deadlock errors (see <xref linkend="failures-and-retries"/>
-  for more information).
+  The next line reports the number of failed transactions (see
+  <xref linkend="failures-and-retries"/> for more information).
   The last line reports the number of transactions per second.
  </para>
 
@@ -790,6 +789,9 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
          <listitem>
           <para>deadlock failures;</para>
          </listitem>
+         <listitem>
+          <para>other failures;</para>
+         </listitem>
         </itemizedlist>
         See <xref linkend="failures-and-retries"/> for more information.
        </para>
@@ -914,6 +916,26 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
       </listitem>
      </varlistentry>
 
+     <varlistentry id="pgbench-option-continue-on-error">
+      <term><option>--continue-on-error</option></term>
+      <listitem>
+       <para>
+        Allows clients to continue running even if an SQL statement fails due to
+        errors other than serialization or deadlock. Unlike serialization and deadlock
+        failures, clients do not retry the same transactions but proceed to the next
+        transaction. This option is useful when your custom script may raise errors for
+        reasons such as unique constraints violation. Without this option, the
+        client is aborted after such errors.
+       </para>
+       <para>
+        Note that serialization and deadlock failures never cause the client to be
+        aborted even after clients retries <option>--max-tries</option> times by
+        default, so they are not affected by this option.
+        See <xref linkend="failures-and-retries"/> for more information.
+       </para>
+      </listitem>
+     </varlistentry>
+
     </variablelist>
    </para>
 
@@ -2408,8 +2430,8 @@ END;
    will be reported as <literal>failed</literal>. If you use the
    <option>--failures-detailed</option> option, the
    <replaceable>time</replaceable> of the failed transaction will be reported as
-   <literal>serialization</literal> or
-   <literal>deadlock</literal> depending on the type of failure (see
+   <literal>serialization</literal>, <literal>deadlock</literal>, or
+   <literal>other</literal> depending on the type of failure (see
    <xref linkend="failures-and-retries"/> for more information).
   </para>
 
@@ -2637,6 +2659,16 @@ END;
       </para>
      </listitem>
     </varlistentry>
+
+    <varlistentry>
+     <term><replaceable>other_sql_failures</replaceable></term>
+     <listitem>
+      <para>
+       number of transactions that got an SQL error
+       (zero unless <option>--failures-detailed</option> is specified)
+      </para>
+     </listitem>
+    </varlistentry>
    </variablelist>
   </para>
 
@@ -2645,8 +2677,8 @@ END;
 <screen>
 <userinput>pgbench --aggregate-interval=10 --time=20 --client=10 --log --rate=1000 --latency-limit=10 --failures-detailed --max-tries=10 test</userinput>
 
-1650260552 5178 26171317 177284491527 1136 44462 2647617 7321113867 0 9866 64 7564 28340 4148 0
-1650260562 4808 25573984 220121792172 1171 62083 3037380 9666800914 0 9998 598 7392 26621 4527 0
+1650260552 5178 26171317 177284491527 1136 44462 2647617 7321113867 0 9866 64 7564 28340 4148 0 0
+1650260562 4808 25573984 220121792172 1171 62083 3037380 9666800914 0 9998 598 7392 26621 4527 0 0
 </screen>
   </para>
 
@@ -2850,10 +2882,20 @@ statement latencies in milliseconds, failures and retries:
   <para>
    A client's run is aborted in case of a serious error; for example, the
    connection with the database server was lost or the end of script was reached
-   without completing the last transaction. In addition, if execution of an SQL
-   or meta command fails for reasons other than serialization or deadlock errors,
-   the client is aborted. Otherwise, if an SQL command fails with serialization or
-   deadlock errors, the client is not aborted. In such cases, the current
+   without completing the last transaction.  The client also aborts
+   if a meta command fails, or if an SQL command fails for reasons other than
+   serialization or deadlock errors when <option>--continue-on-error</option>
+   is not specified.  With <option>--continue-on-error</option>,
+   the client does not abort on such SQL errors and instead proceeds to
+   the next transaction.  These cases are reported as
+   <literal>other failures</literal> in the output.  If the error occurs
+   in a meta command, however, the client still aborts even when this option
+   is specified.
+  </para>
+  <para>
+   If an SQL command fails due to serialization or deadlock errors, the
+   client does not abort, regardless of whether
+   <option>--continue-on-error</option> is used.  Instead, the current
    transaction is rolled back, which also includes setting the client variables
    as they were before the run of this transaction (it is assumed that one
    transaction script contains only one transaction; see
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index a84c68705de..18bce17a245 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -402,8 +402,10 @@ typedef struct StatsData
 	 *   directly successful transactions (they were successfully completed on
 	 *                                     the first try).
 	 *
-	 * A failed transaction is defined as unsuccessfully retried transactions.
-	 * It can be one of two types:
+	 * A failed transaction is counted differently depending on whether
+	 * the --continue-on-error option is specified.
+	 *
+	 * Without --continue-on-error:
 	 *
 	 * failed (the number of failed transactions) =
 	 *   'serialization_failures' (they got a serialization error and were not
@@ -411,6 +413,13 @@ typedef struct StatsData
 	 *   'deadlock_failures' (they got a deadlock error and were not
 	 *                        successfully retried).
 	 *
+	 * With --continue-on-error:
+	 *
+	 * failed (number of failed transactions) =
+	 *   'serialization_failures' + 'deadlock_failures' +
+	 *   'other_sql_failures'  (they got some other SQL error; the transaction was
+	 * not retried and counted as failed due to --continue-on-error).
+	 *
 	 * If the transaction was retried after a serialization or a deadlock
 	 * error this does not guarantee that this retry was successful. Thus
 	 *
@@ -440,6 +449,11 @@ typedef struct StatsData
 	int64		deadlock_failures;	/* number of transactions that were not
 									 * successfully retried after a deadlock
 									 * error */
+	int64		other_sql_failures; /* number of failed transactions for
+									 * reasons other than
+									 * serialization/deadlock failure, which
+									 * is counted if --continue-on-error is
+									 * specified */
 	SimpleStats latency;
 	SimpleStats lag;
 } StatsData;
@@ -770,6 +784,7 @@ static int64 total_weight = 0;
 static bool verbose_errors = false; /* print verbose messages of all errors */
 
 static bool exit_on_abort = false;	/* exit when any client is aborted */
+static bool continue_on_error = false;	/* continue after errors */
 
 /* Builtin test scripts */
 typedef struct BuiltinScript
@@ -954,6 +969,7 @@ usage(void)
 		   "  --log-prefix=PREFIX      prefix for transaction time log file\n"
 		   "                           (default: \"pgbench_log\")\n"
 		   "  --max-tries=NUM          max number of tries to run transaction (default: 1)\n"
+		   "  --continue-on-error      continue running after an SQL error\n"
 		   "  --progress-timestamp     use Unix epoch timestamps for progress\n"
 		   "  --random-seed=SEED       set random seed (\"time\", \"rand\", integer)\n"
 		   "  --sampling-rate=NUM      fraction of transactions to log (e.g., 0.01 for 1%%)\n"
@@ -1467,6 +1483,7 @@ initStats(StatsData *sd, pg_time_usec_t start)
 	sd->retried = 0;
 	sd->serialization_failures = 0;
 	sd->deadlock_failures = 0;
+	sd->other_sql_failures = 0;
 	initSimpleStats(&sd->latency);
 	initSimpleStats(&sd->lag);
 }
@@ -1516,6 +1533,9 @@ accumStats(StatsData *stats, bool skipped, double lat, double lag,
 		case ESTATUS_DEADLOCK_ERROR:
 			stats->deadlock_failures++;
 			break;
+		case ESTATUS_OTHER_SQL_ERROR:
+			stats->other_sql_failures++;
+			break;
 		default:
 			/* internal error which should never occur */
 			pg_fatal("unexpected error status: %d", estatus);
@@ -3367,7 +3387,7 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 			case PGRES_FATAL_ERROR:
 				st->estatus = getSQLErrorStatus(PQresultErrorField(res,
 																   PG_DIAG_SQLSTATE));
-				if (canRetryError(st->estatus))
+				if (continue_on_error || canRetryError(st->estatus))
 				{
 					if (verbose_errors)
 						commandError(st, errmsg);
@@ -4032,7 +4052,10 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
 					if (PQpipelineStatus(st->con) != PQ_PIPELINE_ON)
 						st->state = CSTATE_END_COMMAND;
 				}
-				else if (canRetryError(st->estatus))
+				else if (PQstatus(st->con) == CONNECTION_BAD)
+					st->state = CSTATE_ABORTED;
+				else if ((st->estatus == ESTATUS_OTHER_SQL_ERROR && continue_on_error) ||
+						 canRetryError(st->estatus))
 					st->state = CSTATE_ERROR;
 				else
 					st->state = CSTATE_ABORTED;
@@ -4553,7 +4576,8 @@ static int64
 getFailures(const StatsData *stats)
 {
 	return (stats->serialization_failures +
-			stats->deadlock_failures);
+			stats->deadlock_failures +
+			stats->other_sql_failures);
 }
 
 /*
@@ -4573,6 +4597,8 @@ getResultString(bool skipped, EStatus estatus)
 				return "serialization";
 			case ESTATUS_DEADLOCK_ERROR:
 				return "deadlock";
+			case ESTATUS_OTHER_SQL_ERROR:
+				return "other";
 			default:
 				/* internal error which should never occur */
 				pg_fatal("unexpected error status: %d", estatus);
@@ -4628,6 +4654,7 @@ doLog(TState *thread, CState *st,
 			int64		skipped = 0;
 			int64		serialization_failures = 0;
 			int64		deadlock_failures = 0;
+			int64		other_sql_failures = 0;
 			int64		retried = 0;
 			int64		retries = 0;
 
@@ -4668,10 +4695,12 @@ doLog(TState *thread, CState *st,
 			{
 				serialization_failures = agg->serialization_failures;
 				deadlock_failures = agg->deadlock_failures;
+				other_sql_failures = agg->other_sql_failures;
 			}
-			fprintf(logfile, " " INT64_FORMAT " " INT64_FORMAT,
+			fprintf(logfile, " " INT64_FORMAT " " INT64_FORMAT " " INT64_FORMAT,
 					serialization_failures,
-					deadlock_failures);
+					deadlock_failures,
+					other_sql_failures);
 
 			fputc('\n', logfile);
 
@@ -6310,6 +6339,7 @@ printProgressReport(TState *threads, int64 test_start, pg_time_usec_t now,
 		cur.serialization_failures +=
 			threads[i].stats.serialization_failures;
 		cur.deadlock_failures += threads[i].stats.deadlock_failures;
+		cur.other_sql_failures += threads[i].stats.other_sql_failures;
 	}
 
 	/* we count only actually executed transactions */
@@ -6452,7 +6482,8 @@ printResults(StatsData *total,
 
 	/*
 	 * Remaining stats are nonsensical if we failed to execute any xacts due
-	 * to others than serialization or deadlock errors
+	 * to other than serialization or deadlock errors and --continue-on-error
+	 * is not set.
 	 */
 	if (total_cnt <= 0)
 		return;
@@ -6468,6 +6499,9 @@ printResults(StatsData *total,
 		printf("number of deadlock failures: " INT64_FORMAT " (%.3f%%)\n",
 			   total->deadlock_failures,
 			   100.0 * total->deadlock_failures / total_cnt);
+		printf("number of other failures: " INT64_FORMAT " (%.3f%%)\n",
+			   total->other_sql_failures,
+			   100.0 * total->other_sql_failures / total_cnt);
 	}
 
 	/* it can be non-zero only if max_tries is not equal to one */
@@ -6571,6 +6605,10 @@ printResults(StatsData *total,
 							   sstats->deadlock_failures,
 							   (100.0 * sstats->deadlock_failures /
 								script_total_cnt));
+						printf(" - number of other failures: " INT64_FORMAT " (%.3f%%)\n",
+							   sstats->other_sql_failures,
+							   (100.0 * sstats->other_sql_failures /
+								script_total_cnt));
 					}
 
 					/*
@@ -6730,6 +6768,7 @@ main(int argc, char **argv)
 		{"verbose-errors", no_argument, NULL, 15},
 		{"exit-on-abort", no_argument, NULL, 16},
 		{"debug", no_argument, NULL, 17},
+		{"continue-on-error", no_argument, NULL, 18},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -7083,6 +7122,10 @@ main(int argc, char **argv)
 			case 17:			/* debug */
 				pg_logging_increase_verbosity();
 				break;
+			case 18:			/* continue-on-error */
+				benchmarking_option_set = true;
+				continue_on_error = true;
+				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -7438,6 +7481,7 @@ main(int argc, char **argv)
 		stats.retried += thread->stats.retried;
 		stats.serialization_failures += thread->stats.serialization_failures;
 		stats.deadlock_failures += thread->stats.deadlock_failures;
+		stats.other_sql_failures += thread->stats.other_sql_failures;
 		latency_late += thread->latency_late;
 		conn_total_duration += thread->conn_duration;
 
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 7dd78940300..3c19a36a005 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -1813,6 +1813,28 @@ update counter set i = i+1 returning i \gset
 # Clean up
 $node->safe_psql('postgres', 'DROP TABLE counter;');
 
+# Test --continue-on-error
+$node->safe_psql('postgres',
+	'CREATE TABLE unique_table(i int unique);');
+
+$node->pgbench(
+	'-n -t 10 --continue-on-error --failures-detailed',
+	0,
+	[
+		qr{processed: 1/10\b},
+		qr{other failures: 9\b}
+	],
+	[],
+	'test --continue-on-error',
+	{
+		'001_continue_on_error' => q{
+		INSERT INTO unique_table VALUES(0);
+		}
+	});
+
+# Clean up
+$node->safe_psql('postgres', 'DROP TABLE unique_table;');
+
 # done
 $node->safe_psql('postgres', 'DROP TABLESPACE regress_pgbench_tap_1_ts');
 $node->stop;
-- 
2.43.0

>From 8e4b9d2489f12d342b3a613466dfa043130e8e5f Mon Sep 17 00:00:00 2001
From: Yugo Nagata <[email protected]>
Date: Fri, 26 Sep 2025 10:41:34 +0900
Subject: [PATCH v15 1/3] pgbench: Do not reference error message after another
 PQgetResult() call

Previously, readCommandResponse() accessed the error message
after calling another PQgetResult() to peek at the next result
in order to determine whether the current one was the last.

This caused the error message to be lost in pipeline mode.
Although this issue has never been observed in non-pipeline mode,
referencing an error message after another PQgetResult() call
does not seem like a good idea in general.

Fix this by saving the previous error message and using it for reporting.
---
 src/bin/pgbench/pgbench.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index cc03af05447..a84c68705de 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3272,6 +3272,7 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 	PGresult   *res;
 	PGresult   *next_res;
 	int			qrynum = 0;
+	char	   *errmsg;
 
 	/*
 	 * varprefix should be set only with \gset or \aset, and \endpipeline and
@@ -3287,6 +3288,9 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 	{
 		bool		is_last;
 
+		/* save the previous error message before peek at the next result */
+		errmsg = pg_strdup(PQerrorMessage(st->con));
+
 		/* peek at the next result to know whether the current is last */
 		next_res = PQgetResult(st->con);
 		is_last = (next_res == NULL);
@@ -3356,7 +3360,7 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 				st->num_syncs--;
 				if (st->num_syncs == 0 && PQexitPipelineMode(st->con) != 1)
 					pg_log_error("client %d failed to exit pipeline mode: %s", st->id,
-								 PQerrorMessage(st->con));
+								 errmsg);
 				break;
 
 			case PGRES_NONFATAL_ERROR:
@@ -3366,7 +3370,7 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 				if (canRetryError(st->estatus))
 				{
 					if (verbose_errors)
-						commandError(st, PQerrorMessage(st->con));
+						commandError(st, errmsg);
 					goto error;
 				}
 				/* fall through */
@@ -3374,14 +3378,14 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 			default:
 				/* anything else is unexpected */
 				pg_log_error("client %d script %d aborted in command %d query %d: %s",
-							 st->id, st->use_file, st->command, qrynum,
-							 PQerrorMessage(st->con));
+							 st->id, st->use_file, st->command, qrynum, errmsg);
 				goto error;
 		}
 
 		PQclear(res);
 		qrynum++;
 		res = next_res;
+		pg_free(errmsg);
 	}
 
 	if (qrynum == 0)
@@ -3395,6 +3399,7 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
 error:
 	PQclear(res);
 	PQclear(next_res);
+	pg_free(errmsg);
 	do
 	{
 		res = PQgetResult(st->con);
-- 
2.43.0

Reply via email to