Re: Suggestion to add --continue-client-on-abort option to pgbench

> On Nov 5, 2025, at 23:12, Fujii Masao <masao.fujii@gmail.com> wrote:
> 
> On Wed, Oct 29, 2025 at 1:00 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>> 
>> On Mon, Oct 27, 2025 at 6:13 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>>> One approach to address this issue is to keep calling PQgetResult() until
>>> it returns NULL, and then check the connection status when getSQLErrorStatus()
>>> determines the error state. If the connection status is CONNECTION_BAD
>>> at that point, we can treat it as a connection failure and stop processing
>>> even when --continue-on-error is specified. Attached is a WIP patch
>>> implementing this idea based on the v17 patch. It still needs more testing,
>>> review, and possibly documentation updates.
>>> 
>>> Another option would be to explicitly list all SQLSTATE codes (e.g., 57P01)
>>> that should prevent continued processing, even with --continue-on-error,
>>> inside getSQLErrorStatus(). However, maintaining such a list would be
>>> cumbersome, so I believe the first approach is preferable. Thought?
>> 
>> Nagata-san let me know off-list that there was the case where the previous
>> patch didn't work correctly in pipeline mode. I've updated the patch so that
>> --continue-on-error now works properly in that mode, and also revised
>> the commit message. Updated patch attached.
> 
> In v19 patch, the description of --continue-on-error was placed right after
> --verbose-errors in the docs. Since pgbench long option descriptions are listed
> in alphabetical order, I've moved it to follow --aggregate-interval instead.
> I've also refined the wording of the --continue-on-error description.
> 
> Attached is the updated patch. Unless there are any objections, I will
> commit it.
> 
> Regards,
> 
> -- 
> Fujii Masao
> <v20-0001-pgbench-Add-continue-on-error-option.patch>


I just eyeball reviewed v20 and got a doubt:

```
+static void
+discardAvailableResults(CState *st)
+{
+	PGresult   *res = NULL;
+
+	for (;;)
+	{
+		res = PQgetResult(st->con);
+
+		/*
+		 * Read and discard results until PQgetResult() returns NULL (no more
+		 * results) or a connection failure is detected. If the pipeline
+		 * status is PQ_PIPELINE_ABORTED, more results may still be available
+		 * even after PQgetResult() returns NULL, so continue reading in that
+		 * case.
+		 */
+		if ((res == NULL && PQpipelineStatus(st->con) != PQ_PIPELINE_ABORTED) ||
+			PQstatus(st->con) == CONNECTION_BAD)
+			break;
+
+		PQclear(res);
+	}
+	PQclear(res);
+}
```

If pipeline is aborted and no more results, then the “if” will be "true && false”. And in this case, I guess PQstatus(st->con) != CONNECTION_BAD because it’s not a connection error, then overall, the “if” will be “false”, and it falls into an infinite loop.

Expect that, everything else looks good to me.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/