Re: A few message wording/formatting cleanup patches

> On 28 May 2026, at 21:04, Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>
> On 5/28/26 19:19, Daniel Gustafsson wrote:
>>> On 28 May 2026, at 05:16, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote:
>>>
>>> ...
>>>
>>> 0004: 0004-Use-singular-datachecksum-consistently-in-process-na.patch
>>>
>>> In datachecksum_state.c, the launcher process is referred to in two
>>> different ways: "datachecksum launcher" and "datachecksums
>>> launcher". Considering the worker process name, I think the former is
>>> probably the intended one, so this patch makes the naming consistent
>>> accordingly.
>>>
>>> That said, I can also imagine an interpretation where "datachecksums"
>>> was chosen intentionally to refer to the checksum feature or checksum
>>> set as a whole, so I'm not entirely sure whether this should be
>>> considered a real issue or just a stylistic inconsistency.
>>>
>>> Still, having both forms coexist seems somewhat error-prone in
>>> practice, especially when typing or searching symbol names.
>>
>> I am clearly biased, or Stockholm syndromed, but DataChecksumsXXX was chosen
>> deliberately since it affects the feature which is exposed with the GUC
>> data_checksums. Renaming it does not improve clarity IMHO. The singular form
>> "checksum" is used where it refers to a single entity, like the cluster state
>> which can only be a single value.
>>
>> It would however be an improvement to rename the "datachecksum launcher|worker"
>> cases you found to "datachecksums" since they are user facing.
>>
>> - * This creates the list of databases for the datachecksumsworker workers to
>> + * This creates the list of databases for the datachecksum workers to
>>
>> This comment refers to a worker process of the type datachecksumsworker, the
>> proposed change makes that less clear IMO. That said, the original wording
>> isn't great so maybe "datachecksumsworker process" is better?
>
> My vote would be to use the plural "data checksums" almost everywhere,
> except for the places that actually manipulates a single data checksum.
> Because the feature is "data checksums" with GUC "data_checksums", we
> manipulate all checksums in the cluster, and so on.
>
> So it'd be "datachecksums_state.c" and "datachecksums launcher" (which
> would make it more consistent with the existing injection points, named
> "datachecksumsworker-launcher-..." etc.).

I've pushed the worker/launcher rename now. I held off on renaming the file
for now, I'm not entirely convinced that's an improvement.

--
Daniel Gustafsson