Re: Suggestion to add --continue-client-on-abort option to pgbench - Mailing list pgsql-hackers

> 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
fallsinto an infinite loop. 

Expect that, everything else looks good to me.

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