On Tue, Sep 30, 2025 at 3:17 PM Yugo Nagata <[email protected]> wrote:
>
> On Tue, 30 Sep 2025 13:46:11 +0900
> Fujii Masao <[email protected]> wrote:
>
> > On Tue, Sep 30, 2025 at 10:24 AM Yugo Nagata <[email protected]> wrote:
> > > Fujii-san, thank you for committing the patch that fixes the assertion
> > > failure.
> > > I've attached the remaining patches so that cfbot stays green.
> >
> > Thanks for reattaching the patches!
> >
> > For 0001, after reading the docs on PQresultErrorMessage(), I wonder if it
> > would
> > be better to just use that to get the error message. Thought?
>
> Thank you for your suggestion.
>
> I agree that it is better to use PQresultErrorMessage().
> I had overlooked the existence of this interface.
>
> I've attached the updated patches.
Thanks for updating the patches! I've pushed 0001.
Regarding 0002:
- if (canRetryError(st->estatus))
+ if (continue_on_error || canRetryError(st->estatus))
{
if (verbose_errors)
commandError(st, PQresultErrorMessage(res));
goto error;
With this change, even non-SQL errors (e.g., connection failures) would
satisfy the condition when --continue-on-error is set. Isn't that a problem?
Shouldn't we also check that the error status is one that
--continue-on-error is meant to handle?
+ * Without --continue-on-error:
*
* failed (the number of failed transactions) =
* 'serialization_failures' (they got a serialization error and were not
* successfully retried) +
* 'deadlock_failures' (they got a deadlock error and were not
* successfully retried).
*
+ * With --continue-on-error:
+ *
+ * failed (number of failed transactions) =
+ * 'serialization_failures' + 'deadlock_failures' +
+ * 'other_sql_failures' (they got some other SQL error; the transaction was
+ * not retried and counted as failed due to --continue-on-error).
About the comments on failed transactions: I don't think we need
to split them into separate "with/without --continue-on-error" sections.
How about simplifying them like this?
------------------------
* failed (the number of failed transactions) =
* 'serialization_failures' (they got a serialization error and were not
* successfully retried) +
* 'deadlock_failures' (they got a deadlock error and were not
* successfully retried) +
* 'other_sql_failures' (they failed on the first try or after retries
* due to a SQL error other than serialization or
* deadlock; they are counted as a failed transaction
* only when --continue-on-error is specified).
------------------------
* 'retried' (number of all retried transactions) =
* successfully retried transactions +
* failed transactions.
Since transactions that failed on the first try (i.e., no retries) due to
an SQL error are not counted as 'retried', shouldn't this source comment
be updated?
Regards,
--
Fujii Masao
