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

Hi,

On 2025/07/16 22:49, Yugo Nagata wrote:
>> I think we should also change the error message in pg_log_error. I modified the
>> patch v8-0003 as follows:
>> @@ -3383,8 +3383,8 @@ 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,
>> +                               pg_log_error("client %d aborted in command %d
>> query %d of script %d: %s",
>> +                                                        st->id, st->command,
>> qrynum, st->use_file,
>>                                                          PQerrorMessage(st->con));
>>                                 goto error;
>>                 }
>>
>> With this change, the output now is like this:
>>> pgbench: error: client 0 aborted in command 1 query 0 of script 0: ERROR:
>> duplicate key value violates unique constraint "test_col2_key"
>>
>> I want hear your thoughts.
>
> My idea is to modify this as follows;
>
>                         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));
> +                               commandFailed(st, "SQL", PQerrorMessage(st->con));
>                                 goto error;
>                 }
>
> This fix is originally planned to be included in patch v8, but was missed.
> It is now included in the attached patch, v10.
>
> With this change, the output becomes:
>
>  pgbench: error: client 0 aborted in command 0 (SQL) of script 0;
>   ERROR:  duplicate key value violates unique constraint "t2_pkey"
>
> Although there is a slight difference, the message is essentially the same as
> your proposal. Also, I believe the use of commandFailed() makes the code simpler
> and more consistent.
>
> What do you think?
>

Thank you for the new patch! I think Nagata-san's v10 patch is a clear
improvement over my v9 patch. I'm happy with the changes.


>> Also, let me ask one question. In this case, I directly modified your commit in
>> the v8-0003 patch. Is that the right way to update the patch?
>
> I’m not sure if that’s the best way, but I think modifying the patch directly is a
> valid way to propose an alternative approach during discussion, as long as the original
> patch is respected. It can often help clarify suggestions.

I understand that. Thank you.

Regards,
Rintaro Ikeda