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.
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 |