> On Nov 5, 2025, at 23:12, Fujii Masao <[email protected]> wrote:
> 
> On Wed, Oct 29, 2025 at 1:00 AM Fujii Masao <[email protected]> wrote:
>> 
>> On Mon, Oct 27, 2025 at 6:13 PM Fujii Masao <[email protected]> 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/






Reply via email to