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
