Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Make property graph object descriptions better translatable

  2. Constistent naming for datacheckusms processes

  3. libpq: Fix grease error message style

  1. A few message wording/formatting cleanup patches

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-05-28T03:16:22Z

    Hello,

While translating error messages into Japanese, I came across some
unusually formatted messages and messages that are difficult to
translate naturally into Japanese.

0001: 0001-Make-propgraph-object-descriptions-translatable.patch

getObjectDescription() currently builds the object name for
PropgraphElementRelationId incrementally with appendStringInfo(), but
that works poorly with Japanese because it effectively fixes the word
order.  We probably need to construct the whole name from a single
format string instead.


0002: 0002-Use-double-quotes-in-message.patch

fe-protocol3.c has the following message containing backquotes:

> libpq_append_conn_error(conn,
> "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");

but that doesn't seem to match our usual style.  Nearby messages use
double quotes instead.


0003: 0003-Add-missing-period-to-HINT-message.patch

In be-secure-openssl.c, the following HINT message is missing a
trailing period:

> errhint("If ssl_sni is enabled then add configuration to "%s", else "%s"",


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.

Regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  2. Re: A few message wording/formatting cleanup patches

    Daniel Gustafsson <daniel@yesql.se> — 2026-05-28T17:19:19Z

    > On 28 May 2026, at 05:16, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> 
> Hello,
> 
> While translating error messages into Japanese

Thank you so much for doing this!

Some comments on a few of your patches:

> 0002: 0002-Use-double-quotes-in-message.patch
> 
> fe-protocol3.c has the following message containing backquotes:
> 
>> libpq_append_conn_error(conn,
>> "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
> 
> but that doesn't seem to match our usual style.  Nearby messages use
> double quotes instead.

Agreed. Shouldn't it also be adding the parameter as a %s to further match our style?

-  libpq_append_conn_error(conn, "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
+  libpq_append_conn_error(conn, "server did not report the unsupported \"%s\" parameter in its protocol negotiation message", "_pq_.test_protocol_negotiation");

> 0003: 0003-Add-missing-period-to-HINT-message.patch
> 
> In be-secure-openssl.c, the following HINT message is missing a
> trailing period:
> 
>> errhint("If ssl_sni is enabled then add configuration to "%s", else "%s"",

My bad, will fix.  Reading the hint it's also not as helpful as it could be for
a hint I think, perhaps this would be better?

-  errhint("If ssl_sni is enabled then add configuration to \"%s\", else \"%s\"",
+  errhint("If ssl_sni is enabled then add configuration to \"%s\", else configure SSL in \"%s\".",

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

--
Daniel Gustafsson

  3. Re: A few message wording/formatting cleanup patches

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-05-28T17:23:24Z

    On Thu, May 28, 2026 at 10:19 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> > On 28 May 2026, at 05:16, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> >> libpq_append_conn_error(conn,
> >> "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
> >
> > but that doesn't seem to match our usual style.  Nearby messages use
> > double quotes instead.
>
> Agreed. Shouldn't it also be adding the parameter as a %s to further match our style?
>
> -  libpq_append_conn_error(conn, "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
> +  libpq_append_conn_error(conn, "server did not report the unsupported \"%s\" parameter in its protocol negotiation message", "_pq_.test_protocol_negotiation");

+1, will fix.

Thanks!
--Jacob

  4. Re: A few message wording/formatting cleanup patches

    Tomas Vondra <tomas@vondra.me> — 2026-05-28T19:04:40Z

    
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

  5. Re: A few message wording/formatting cleanup patches

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-05-29T18:46:37Z

    On Thu, May 28, 2026 at 10:23 AM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
> +1, will fix.

(Pushed.)

--Jacob

  6. Re: A few message wording/formatting cleanup patches

    Daniel Gustafsson <daniel@yesql.se> — 2026-05-29T20:10:23Z

    > On 28 May 2026, at 21:04, Tomas Vondra <tomas@vondra.me> wrote:
> 
> 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.).

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