A few message wording/formatting cleanup patches

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

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

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

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

+1, will fix.

(Pushed.)

--Jacob

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