Обсуждение: Add missing period to HINT messages

Поиск
Список
Период
Сортировка

Add missing period to HINT messages

От
Peter Smith
Дата:
Hi,

According to the error message style guide [1], "Detail and hint
messages: Use complete sentences, and end each with a period."

I found there is a small group of HINT messages not following that period rule.

PSA a patch that fixes the missing period. In passing, also fixed a
typo in a hint message /msut/must/.

~~~

On further inspection, lots of these affected messages don't even seem
grammatically correct to me.

I did not modify them, but here are examples of what I mean.

e.g.
"ALTER TABLE ... MERGE PARTITIONS can only merge partitions don't have
sub-partitions."
/don't have/that don't have/

e.g.
"ALTER TABLE ... SPLIT PARTITION can only split partitions don't have
sub-partitions."
/don't have/that don't have/

e.g.
"To split DEFAULT partition one of the new partition must be DEFAULT."
missing word? -- "To split the DEFAULT partition"
should be plural? -- "one of the new partitions"

e.g.
"%s require combined bounds of new partitions must exactly match the
bound of the split partition."
/require/requires/ ?
/match the bound/match the bounds/

======
[1] https://www.postgresql.org/docs/devel/error-style-guide.html#ERROR-STYLE-GUIDE-GRAMMAR-PUNCTUATION

Kind Regards,
Peter Smith.
Fujitsu Australia

Вложения

Re: Add missing period to HINT messages

От
Chao Li
Дата:

> On Apr 9, 2026, at 07:30, Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi,
>
> According to the error message style guide [1], "Detail and hint
> messages: Use complete sentences, and end each with a period."
>
> I found there is a small group of HINT messages not following that period rule.
>
> PSA a patch that fixes the missing period. In passing, also fixed a
> typo in a hint message /msut/must/.
>
> ~~~
>
> On further inspection, lots of these affected messages don't even seem
> grammatically correct to me.
>
> I did not modify them, but here are examples of what I mean.
>
> e.g.
> "ALTER TABLE ... MERGE PARTITIONS can only merge partitions don't have
> sub-partitions."
> /don't have/that don't have/
>
> e.g.
> "ALTER TABLE ... SPLIT PARTITION can only split partitions don't have
> sub-partitions."
> /don't have/that don't have/
>
> e.g.
> "To split DEFAULT partition one of the new partition must be DEFAULT."
> missing word? -- "To split the DEFAULT partition"
> should be plural? -- "one of the new partitions"
>
> e.g.
> "%s require combined bounds of new partitions must exactly match the
> bound of the split partition."
> /require/requires/ ?
> /match the bound/match the bounds/
>
> ======
> [1] https://www.postgresql.org/docs/devel/error-style-guide.html#ERROR-STYLE-GUIDE-GRAMMAR-PUNCTUATION
>
> Kind Regards,
> Peter Smith.
> Fujitsu Australia
> <v1-0001-Add-missing-period-to-HINT-messages.patch>

LGTM.

BTW, errdetail should follow the same style, and I see some detail messages miss periods, maybe this patch can include
thoseas well. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/







Re: Add missing period to HINT messages

От
Peter Smith
Дата:
On Thu, Apr 9, 2026 at 12:09 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> > On Apr 9, 2026, at 07:30, Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Hi,
> >
> > According to the error message style guide [1], "Detail and hint
> > messages: Use complete sentences, and end each with a period."
> >
> > I found there is a small group of HINT messages not following that period rule.
> >
> > PSA a patch that fixes the missing period. In passing, also fixed a
> > typo in a hint message /msut/must/.
> >
> > ~~~
> >
> > On further inspection, lots of these affected messages don't even seem
> > grammatically correct to me.
> >
> > I did not modify them, but here are examples of what I mean.
> >
> > e.g.
> > "ALTER TABLE ... MERGE PARTITIONS can only merge partitions don't have
> > sub-partitions."
> > /don't have/that don't have/
> >
> > e.g.
> > "ALTER TABLE ... SPLIT PARTITION can only split partitions don't have
> > sub-partitions."
> > /don't have/that don't have/
> >
> > e.g.
> > "To split DEFAULT partition one of the new partition must be DEFAULT."
> > missing word? -- "To split the DEFAULT partition"
> > should be plural? -- "one of the new partitions"
> >
> > e.g.
> > "%s require combined bounds of new partitions must exactly match the
> > bound of the split partition."
> > /require/requires/ ?
> > /match the bound/match the bounds/
> >
> > ======
> > [1] https://www.postgresql.org/docs/devel/error-style-guide.html#ERROR-STYLE-GUIDE-GRAMMAR-PUNCTUATION
> >
> > Kind Regards,
> > Peter Smith.
> > Fujitsu Australia
> > <v1-0001-Add-missing-period-to-HINT-messages.patch>
>
> LGTM.
>
> BTW, errdetail should follow the same style, and I see some detail messages miss periods, maybe this patch can
includethose as well. 
>

Yep. It is already under way. I will post the equivalent errdetail
patch shortly. (as soon as it passes running make- check-world).

======
Kind Regards,
Peter Smith.
Fujitsu Australia



Re: Add missing period to HINT messages

От
Chao Li
Дата:

> On Apr 9, 2026, at 11:24, Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Thu, Apr 9, 2026 at 12:09 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Apr 9, 2026, at 07:30, Peter Smith <smithpb2250@gmail.com> wrote:
>>>
>>> Hi,
>>>
>>> According to the error message style guide [1], "Detail and hint
>>> messages: Use complete sentences, and end each with a period."
>>>
>>> I found there is a small group of HINT messages not following that period rule.
>>>
>>> PSA a patch that fixes the missing period. In passing, also fixed a
>>> typo in a hint message /msut/must/.
>>>
>>> ~~~
>>>
>>> On further inspection, lots of these affected messages don't even seem
>>> grammatically correct to me.
>>>
>>> I did not modify them, but here are examples of what I mean.
>>>
>>> e.g.
>>> "ALTER TABLE ... MERGE PARTITIONS can only merge partitions don't have
>>> sub-partitions."
>>> /don't have/that don't have/
>>>
>>> e.g.
>>> "ALTER TABLE ... SPLIT PARTITION can only split partitions don't have
>>> sub-partitions."
>>> /don't have/that don't have/
>>>
>>> e.g.
>>> "To split DEFAULT partition one of the new partition must be DEFAULT."
>>> missing word? -- "To split the DEFAULT partition"
>>> should be plural? -- "one of the new partitions"
>>>
>>> e.g.
>>> "%s require combined bounds of new partitions must exactly match the
>>> bound of the split partition."
>>> /require/requires/ ?
>>> /match the bound/match the bounds/
>>>
>>> ======
>>> [1] https://www.postgresql.org/docs/devel/error-style-guide.html#ERROR-STYLE-GUIDE-GRAMMAR-PUNCTUATION
>>>
>>> Kind Regards,
>>> Peter Smith.
>>> Fujitsu Australia
>>> <v1-0001-Add-missing-period-to-HINT-messages.patch>
>>
>> LGTM.
>>
>> BTW, errdetail should follow the same style, and I see some detail messages miss periods, maybe this patch can
includethose as well. 
>>
>
> Yep. It is already under way. I will post the equivalent errdetail
> patch shortly. (as soon as it passes running make- check-world).
>
> ======
> Kind Regards,
> Peter Smith.
> Fujitsu Australia

I am not sure if your scope includes contrib/, if yes, I am sure you will find some occurrences there.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/







Re: Add missing period to HINT messages

От
Peter Smith
Дата:
On Thu, Apr 9, 2026 at 1:34 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
...
>
> I am not sure if your scope includes contrib/, if yes, I am sure you will find some occurrences there.
>

Updated one more message found in contrib.

PSA v2.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Вложения

Re: Add missing period to HINT messages

От
Robert Treat
Дата:
On Thu, Apr 9, 2026 at 9:05 PM Peter Smith <smithpb2250@gmail.com> wrote:
> On Thu, Apr 9, 2026 at 1:34 PM Chao Li <li.evan.chao@gmail.com> wrote:
> >
> ...
> >
> > I am not sure if your scope includes contrib/, if yes, I am sure you will find some occurrences there.
> >
>
> Updated one more message found in contrib.
>
> PSA v2.
>

+1 to the general idea here, although at the risk of looking at
patches before the morning coffee has settled in, ISTM you might have
missed some entries? And/or this patch isn't against HEAD?  For
example, you seem to be catching the line here

https://github.com/postgres/postgres/blame/009ea1b08d7b8843435bd0f1137fa3df09aac79f/src/test/regress/expected/partition_split.out#L60,
but not the one on line 52, maybe because it looks like a comment (but
istm we should clean these all up. no?)

Also FWIW, it might seem a little weird not to clean up the grammar
issues too, but I think this is the right move, to update these
changes cleanly/separately and do those changes as a separate patch, I
can imagine that some of those will require more futzing, for example

> "To split DEFAULT partition one of the new partition must be DEFAULT."
> missing word? -- "To split the DEFAULT partition"
> should be plural? -- "one of the new partitions"
>

I guess there is a balance wrt terseness, and maybe needs a comma:

"To split DEFAULT partition, one new partition must be DEFAULT."
"To split the DEFAULT partition, one of the new partitions must be DEFAULT."
 "To split a DEFAULT partition, one of the new partitions must be
created as DEFAULT."


Robert Treat
https://xzilla.net



Re: Add missing period to HINT messages

От
Peter Smith
Дата:
On Fri, Apr 10, 2026 at 11:10 PM Robert Treat <rob@xzilla.net> wrote:
>
> On Thu, Apr 9, 2026 at 9:05 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > On Thu, Apr 9, 2026 at 1:34 PM Chao Li <li.evan.chao@gmail.com> wrote:
> > >
> > ...
> > >
> > > I am not sure if your scope includes contrib/, if yes, I am sure you will find some occurrences there.
> > >
> >
> > Updated one more message found in contrib.
> >
> > PSA v2.
> >
>
> +1 to the general idea here, although at the risk of looking at
> patches before the morning coffee has settled in, ISTM you might have
> missed some entries? And/or this patch isn't against HEAD?  For
> example, you seem to be catching the line here
>
https://github.com/postgres/postgres/blame/009ea1b08d7b8843435bd0f1137fa3df09aac79f/src/test/regress/expected/partition_split.out#L60,
> but not the one on line 52, maybe because it looks like a comment (but
> istm we should clean these all up. no?)
>

Thanks for your review!

Yes, the patch applies to HEAD. It looks like I was a bit slack in
updating some test comments. Hopefully, I have found them all now.

> Also FWIW, it might seem a little weird not to clean up the grammar
> issues too, but I think this is the right move, to update these
> changes cleanly/separately and do those changes as a separate patch, I
> can imagine that some of those will require more futzing, for example
>

Yes, the idea was just to fight one battle at a time.

PSA v3.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Вложения

Re: Add missing period to HINT messages

От
Amit Kapila
Дата:
On Mon, Apr 13, 2026 at 6:32 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Fri, Apr 10, 2026 at 11:10 PM Robert Treat <rob@xzilla.net> wrote:
> >
> > On Thu, Apr 9, 2026 at 9:05 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > > On Thu, Apr 9, 2026 at 1:34 PM Chao Li <li.evan.chao@gmail.com> wrote:
> > > >
> > > ...
> > > >
> > > > I am not sure if your scope includes contrib/, if yes, I am sure you will find some occurrences there.
> > > >
> > >
> > > Updated one more message found in contrib.
> > >
> > > PSA v2.
> > >
> >
> > +1 to the general idea here, although at the risk of looking at
> > patches before the morning coffee has settled in, ISTM you might have
> > missed some entries? And/or this patch isn't against HEAD?  For
> > example, you seem to be catching the line here
> >
https://github.com/postgres/postgres/blame/009ea1b08d7b8843435bd0f1137fa3df09aac79f/src/test/regress/expected/partition_split.out#L60,
> > but not the one on line 52, maybe because it looks like a comment (but
> > istm we should clean these all up. no?)
> >
>
> Thanks for your review!
>
> Yes, the patch applies to HEAD. It looks like I was a bit slack in
> updating some test comments. Hopefully, I have found them all now.
>

BTW, I find such a code cleanup exercise can be done even after
feature freeze. If so, shall we do it as a HEAD-only patch or do it in
bank branches as well?

--
With Regards,
Amit Kapila.



Re: Add missing period to HINT messages

От
Robert Treat
Дата:
On Mon, Apr 13, 2026 at 4:28 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Mon, Apr 13, 2026 at 6:32 AM Peter Smith <smithpb2250@gmail.com> wrote:
> > On Fri, Apr 10, 2026 at 11:10 PM Robert Treat <rob@xzilla.net> wrote:
> > >
> > > On Thu, Apr 9, 2026 at 9:05 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > > > On Thu, Apr 9, 2026 at 1:34 PM Chao Li <li.evan.chao@gmail.com> wrote:
> > > > >
> > > > ...
> > > > >
> > > > > I am not sure if your scope includes contrib/, if yes, I am sure you will find some occurrences there.
> > > > >
> > > >
> > > > Updated one more message found in contrib.
> > > >
> > > > PSA v2.
> > > >
> > >
> > > +1 to the general idea here, although at the risk of looking at
> > > patches before the morning coffee has settled in, ISTM you might have
> > > missed some entries? And/or this patch isn't against HEAD?  For
> > > example, you seem to be catching the line here
> > >
https://github.com/postgres/postgres/blame/009ea1b08d7b8843435bd0f1137fa3df09aac79f/src/test/regress/expected/partition_split.out#L60,
> > > but not the one on line 52, maybe because it looks like a comment (but
> > > istm we should clean these all up. no?)
> > >
> >
> > Thanks for your review!
> >
> > Yes, the patch applies to HEAD. It looks like I was a bit slack in
> > updating some test comments. Hopefully, I have found them all now.
> >
>
> BTW, I find such a code cleanup exercise can be done even after
> feature freeze. If so, shall we do it as a HEAD-only patch or do it in
> bank branches as well?
>

I don't have a strong opinion on it, but I think generally that clean
up patches (where we aren't fixing some kind of document mistake or
misleading information) generally just go into HEAD for the next
release, though if it easily applies cleanly to back branches and you
want to do it, it's probably ok to apply it backwards, but I wouldn't
spend any time on it if it didn't.

Robert Treat
https://xzilla.net



Re: Add missing period to HINT messages

От
"David G. Johnston"
Дата:
On Monday, April 13, 2026, Robert Treat <rob@xzilla.net> wrote:


I don't have a strong opinion on it, but I think generally that clean
up patches (where we aren't fixing some kind of document mistake or
misleading information) generally just go into HEAD for the next
release, though if it easily applies cleanly to back branches and you
want to do it, it's probably ok to apply it backwards, but I wouldn't
spend any time on it if it didn't.

I was under the impression that on both user-facing behavior changes grounds, and translation grounds, back-patching changes to existing messages is not allowed absent a true factual bug.

David J.

Re: Add missing period to HINT messages

От
Tom Lane
Дата:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> I was under the impression that on both user-facing behavior changes
> grounds, and translation grounds, back-patching changes to existing
> messages is not allowed absent a true factual bug.

"Not allowed" is a bit strong, but yeah we don't usually mess with
translatable strings in back branches without a pretty good reason.
The translators have enough work as it is.

            regards, tom lane



Re: Add missing period to HINT messages

От
Chao Li
Дата:

> On Apr 14, 2026, at 08:39, Robert Treat <rob@xzilla.net> wrote:
>
> On Mon, Apr 13, 2026 at 4:28 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Mon, Apr 13, 2026 at 6:32 AM Peter Smith <smithpb2250@gmail.com> wrote:
>>> On Fri, Apr 10, 2026 at 11:10 PM Robert Treat <rob@xzilla.net> wrote:
>>>>
>>>> On Thu, Apr 9, 2026 at 9:05 PM Peter Smith <smithpb2250@gmail.com> wrote:
>>>>> On Thu, Apr 9, 2026 at 1:34 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>>>>>
>>>>> ...
>>>>>>
>>>>>> I am not sure if your scope includes contrib/, if yes, I am sure you will find some occurrences there.
>>>>>>
>>>>>
>>>>> Updated one more message found in contrib.
>>>>>
>>>>> PSA v2.
>>>>>
>>>>
>>>> +1 to the general idea here, although at the risk of looking at
>>>> patches before the morning coffee has settled in, ISTM you might have
>>>> missed some entries? And/or this patch isn't against HEAD?  For
>>>> example, you seem to be catching the line here
>>>>
https://github.com/postgres/postgres/blame/009ea1b08d7b8843435bd0f1137fa3df09aac79f/src/test/regress/expected/partition_split.out#L60,
>>>> but not the one on line 52, maybe because it looks like a comment (but
>>>> istm we should clean these all up. no?)
>>>>
>>>
>>> Thanks for your review!
>>>
>>> Yes, the patch applies to HEAD. It looks like I was a bit slack in
>>> updating some test comments. Hopefully, I have found them all now.
>>>
>>
>> BTW, I find such a code cleanup exercise can be done even after
>> feature freeze. If so, shall we do it as a HEAD-only patch or do it in
>> bank branches as well?
>>
>
> I don't have a strong opinion on it, but I think generally that clean
> up patches (where we aren't fixing some kind of document mistake or
> misleading information) generally just go into HEAD for the next
> release, though if it easily applies cleanly to back branches and you
> want to do it, it's probably ok to apply it backwards, but I wouldn't
> spend any time on it if it didn't.
>
> Robert Treat
> https://xzilla.net

I don’t see a branch for 19 is cut out, so is HEAD still 19 today?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/







Re: Add missing period to HINT messages

От
Bruce Momjian
Дата:
On Tue, Apr 14, 2026 at 09:10:42AM +0800, Chao Li wrote:
> I don’t see a branch for 19 is cut out, so is HEAD still 19 today?

Yes.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.



Re: Add missing period to HINT messages

От
Tom Lane
Дата:
Chao Li <li.evan.chao@gmail.com> writes:
> I don’t see a branch for 19 is cut out, so is HEAD still 19 today?

Yes.  We typically don't make the branch until after beta1.
It definitely has to happen before the first CF for the new cycle
opens, but up till that point we tend to think it'd just double
the committing work for bug fixes, as well as encourage people
to work on new development at a time they should be working on
stabilizing/testing the release.

In recent years it's tended to happen late June (grep the commit
log for "Stamp HEAD as ...").

            regards, tom lane



Re: Add missing period to HINT messages

От
Peter Smith
Дата:
On Mon, Apr 13, 2026 at 9:28 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Apr 13, 2026 at 6:32 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > On Fri, Apr 10, 2026 at 11:10 PM Robert Treat <rob@xzilla.net> wrote:
> > >
> > > On Thu, Apr 9, 2026 at 9:05 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > > > On Thu, Apr 9, 2026 at 1:34 PM Chao Li <li.evan.chao@gmail.com> wrote:
> > > > >
> > > > ...
> > > > >
> > > > > I am not sure if your scope includes contrib/, if yes, I am sure you will find some occurrences there.
> > > > >
> > > >
> > > > Updated one more message found in contrib.
> > > >
> > > > PSA v2.
> > > >
> > >
> > > +1 to the general idea here, although at the risk of looking at
> > > patches before the morning coffee has settled in, ISTM you might have
> > > missed some entries? And/or this patch isn't against HEAD?  For
> > > example, you seem to be catching the line here
> > >
https://github.com/postgres/postgres/blame/009ea1b08d7b8843435bd0f1137fa3df09aac79f/src/test/regress/expected/partition_split.out#L60,
> > > but not the one on line 52, maybe because it looks like a comment (but
> > > istm we should clean these all up. no?)
> > >
> >
> > Thanks for your review!
> >
> > Yes, the patch applies to HEAD. It looks like I was a bit slack in
> > updating some test comments. Hopefully, I have found them all now.
> >
>
> BTW, I find such a code cleanup exercise can be done even after
> feature freeze. If so, shall we do it as a HEAD-only patch or do it in
> bank branches as well?
>

Thanks for pushing.

======
Kind Regards,
Peter Smith.
Fujitsu Australia.