Re: A few message wording/formatting cleanup patches

On 5/28/26 19:19, Daniel Gustafsson wrote:
>> On 28 May 2026, at 05:16, Kyotaro Horiguchi <horikyota.ntt@gmail.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.).

But this opinion comes from someone who accidentally named an internal
queueing library "queque" and had to live with that shame for years.


regards

-- 
Tomas Vondra