Thank you for reviewing the patches.
On 2025/09/19 20:56, Yugo Nagata wrote:
>>>> A client's run is aborted in case of a serious error; for example, the
>>>> connection with the database server was lost or the end of script was reached
>>>> without completing the last transaction. The client also aborts
>>>> if a meta-command fails, or if an SQL command fails for reasons other than
>>>> serialization or deadlock errors when --continue-on-error is not specified.
>>>> With --continue-on-error, the client does not abort on such SQL errors
>>>> and instead proceeds to the next transaction. These cases are reported
>>>> as "other failures" in the output. If the error occurs in a meta-command,
>>>> however, the client still aborts even when this option is specified.
>>>> ----------------------------
>>>
>>> I'm fine with that. This version is clearer.
I also agree with this.
>>
>> Also I'd like to share the review comments for 0002 and 0003.
>>
>> Regarding 0002:
>>
>> - TSTATUS_OTHER_ERROR,
>> + TSTATUS_UNKNOWN_ERROR,
>>
>> You did this rename to avoid confusion with other_sql_errors.
>> I see the intention, but I'm not sure if this concern is really valid
>> and if the rename adds much value. Also, TSTATUS_UNKNOWN_ERROR
>> might be mistakenly assumed to be related to PQTRANS_UNKNOWN,
>> even though they aren't related...
>
> I don’t have a strong opinion on this, but I think TSTATUS_* is slightly
> related to PQTRANS_*, since getTransactionStatus() determines the TSTATUS
> value based on PQTRANS. There is no one-to-one relationship, of course,
> but it is more related than ESTATUS_OTHER_SQL_ERROR, which is entirely
> separate.
>
>> But if we agree with this change, I think it should be folded into 0001,
>> since there's no strong reason to keep it separate.
>
> +1
>
> I personally don't care if ommiting this change, but I would like to wait
> for Ikeda-san's response because he is the author of these two patches.
>
The points you both raise make sense to me.
Changing the macro name is not important for the purpose of the patch, so I now
feel it would be reasonable to drop patch 0002.
Regards,
Rintaro Ikeda
