Re: Reword messages using "as" instead of "because"

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Reword messages using "as" instead of "because"
Date: 2025-09-16 03:56:17
Message-ID: CAA4eK1K4mqS1uivJGTaFwzKYSsh5Ygrc0hQWs0qY6zAUEorRDA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Sep 16, 2025 at 8:17 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> I noticed that the recent commit 0d48d393d46 introduced the following
> three messages:
>
> 4793> errdetail("Retention is stopped as the apply process is not advancing its xmin within the configured max_retention_duration of %u ms.",
> 4822> ? errdetail("Retention is re-enabled as the apply process is advancing its xmin within the configured max_retention_duration of %u ms.",
> 4824> : errdetail("Retention is re-enabled as max_retention_duration is set to unlimited."));
>
> I think I saw other instances of this kind of as recently, and I
> thought we had agreed to avoid this usage and prefer because instead,
> but I lost track of where that discussion took place.
>
> Anyway, unlike some past uses, these ones are apparently confusing,
> and I'd like to propose changing the wording to because.
>

Thanks for pointing this out. I checked the code and found the use of
'because' is preferred.

> In addition, I felt that the tense in the second message is not
> immediately clear. If it is reasonable and keeps the correct sense,
> I'd like to propose changing "is (not) advancing its xmin within" to
> "has (not) advanced its xmin into".
>
> + errdetail("Retention is stopped because the apply process has not advanced its xmin into the configured max_retention_duration of %u ms.",
> + ? errdetail("Retention is re-enabled because the apply process has advanced its xmin into the configured max_retention_duration of %u ms.",
> + : errdetail("Retention is re-enabled because max_retention_duration is set to unlimited."));
>

In the above sentence, has advanced sounds like we have already
advanced but that is not the case. Also, use of into looks odd to me.
How about changing it to: "Retention is re-enabled because the apply
process can advance its xmin within the configured
max_retention_duration of %u ms."?

Similarly for the first message, how about: "Retention is stopped
because the apply process could not advance its xmin within the
configured max_retention_duration of %u ms."?

--
With Regards,
Amit Kapila.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2025-09-16 04:11:39 Re: Fix missing EvalPlanQual recheck for TID scans
Previous Message Zhijie Hou (Fujitsu) 2025-09-16 03:53:38 RE: Conflict detection for update_deleted in logical replication