Horton
Menu

Thread

  1. Add missing period to DETAIL messages

    Peter Smith <smithpb2250@gmail.com> — 2026-04-09T04:27:32Z

    Hi,

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

I found some DETAIL messages not following that period rule.

PSA a patch that fixes the missing period. In passing, I also fixed
some capitalization.

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

Kind Regards,
Peter Smith.
Fujitsu Australia

  2. Re: Add missing period to DETAIL messages

    Chao Li <li.evan.chao@gmail.com> — 2026-04-09T08:00:10Z

    
> On Apr 9, 2026, at 12:27, 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 some DETAIL messages not following that period rule.
> 
> PSA a patch that fixes the missing period. In passing, I also fixed
> some capitalization.
> 
> ======
> [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-DETAIL-messages.patch>


```
-					 errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long",
+					 errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long.”,
```

First “p” should be capital.

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

  3. Re: Add missing period to DETAIL messages

    Peter Smith <smithpb2250@gmail.com> — 2026-04-10T01:02:23Z

    On Thu, Apr 9, 2026 at 6:00 PM Chao Li <li.evan.chao@gmail.com> wrote:
...
> > <v1-0001-Add-missing-period-to-DETAIL-messages.patch>
>
>
> ```
> -                                        errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long",
> +                                        errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long.”,
> ```
>
> First “p” should be capital.
>

Thanks for your review.

I have fixed that capitalisation, plus a couple more.

PSA v2.

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

  4. Re: Add missing period to DETAIL messages

    Peter Smith <smithpb2250@gmail.com> — 2026-04-13T01:04:50Z

    Found one more oversight.

PSA v3.

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

  5. Re: Add missing period to DETAIL messages

    Peter Smith <smithpb2250@gmail.com> — 2026-04-13T01:05:38Z

    (here's the missing attachment)

On Mon, Apr 13, 2026 at 11:04 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Found one more oversight.
>
> PSA v3.
>
> ======
> Kind Regards,
> Peter Smith.
> Fujitsu Australia

  6. Re: Add missing period to DETAIL messages

    vignesh C <vignesh21@gmail.com> — 2026-04-14T05:51:40Z

    On Mon, 13 Apr 2026 at 06:36, Peter Smith <smithpb2250@gmail.com> wrote:
>
> (here's the missing attachment)
>
> On Mon, Apr 13, 2026 at 11:04 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Found one more oversight.
> >
> > PSA v3.

Should these also be updated:
1) ginfuncs.c
if (opaq->flags != (GIN_DATA | GIN_LEAF | GIN_COMPRESSED))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("input page is not a compressed GIN data leaf page"),
errdetail("Flags %04X, expected %04X",
   opaq->flags,
   (GIN_DATA | GIN_LEAF | GIN_COMPRESSED))));

2) shell_archive.c
ereport(lev,
(errmsg("archive command failed with exit code %d",
WEXITSTATUS(rc)),
errdetail("The failed archive command was: %s",
   xlogarchcmd)));

3) shell_archive.c
ereport(lev,
(errmsg("archive command was terminated by exception 0x%X",
WTERMSIG(rc)),
errhint("See C include file \"ntstatus.h\" for a description of the
hexadecimal value."),
errdetail("The failed archive command was: %s",
   xlogarchcmd)));

4) shell_archive.c
ereport(lev,
(errmsg("archive command was terminated by signal %d: %s",
WTERMSIG(rc), pg_strsignal(WTERMSIG(rc))),
errdetail("The failed archive command was: %s",
   xlogarchcmd)));

5) shell_archive.c
ereport(lev,
(errmsg("archive command exited with unrecognized status %d",
rc),
errdetail("The failed archive command was: %s",
   xlogarchcmd)));

6) matview.c
ereport(ERROR,
(errcode(ERRCODE_CARDINALITY_VIOLATION),
errmsg("new data for materialized view \"%s\" contains duplicate rows
without any null columns",
RelationGetRelationName(matviewRel)),
errdetail("Row: %s",
   SPI_getvalue(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 1))));

Regards,
Vignesh

  7. Re: Add missing period to DETAIL messages

    Peter Smith <smithpb2250@gmail.com> — 2026-04-14T06:28:37Z

    On Tue, Apr 14, 2026 at 3:51 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Mon, 13 Apr 2026 at 06:36, Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > (here's the missing attachment)
> >
> > On Mon, Apr 13, 2026 at 11:04 AM Peter Smith <smithpb2250@gmail.com> wrote:
> > >
> > > Found one more oversight.
> > >
> > > PSA v3.
>

Hi Vignesh.

Thanks for your review!

I deliberately avoided any potentially controversial or troublesome
modifications.

So I have already seen all those you questioned and chose to avoid
them for various reasons.

In general, I did not put a period on any message that ended with a
":" followed by some substitution, for fear that the period (.) might
ambiguously appear to be part of the substituted value. Below are some
other reasons...

> Should these also be updated:
> 1) ginfuncs.c
> if (opaq->flags != (GIN_DATA | GIN_LEAF | GIN_COMPRESSED))
> ereport(ERROR,
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> errmsg("input page is not a compressed GIN data leaf page"),
> errdetail("Flags %04X, expected %04X",
>    opaq->flags,
>    (GIN_DATA | GIN_LEAF | GIN_COMPRESSED))));
>

Yeah. This was one I was wavering about. I didn't want to be worried
that the '.' might somehow mess with the last format specifier. And
escaping the period using "\." just seemed overkill. I concluded there
seemed to be more reasons to avoid changing it than to change it.

> 2) shell_archive.c
> ereport(lev,
> (errmsg("archive command failed with exit code %d",
> WEXITSTATUS(rc)),
> errdetail("The failed archive command was: %s",
>    xlogarchcmd)));

Not done because I didn't want it to appear that the '.' might be part
of the substituted command.

>
> 3) shell_archive.c
> ereport(lev,
> (errmsg("archive command was terminated by exception 0x%X",
> WTERMSIG(rc)),
> errhint("See C include file \"ntstatus.h\" for a description of the
> hexadecimal value."),
> errdetail("The failed archive command was: %s",
>    xlogarchcmd)));

Not done because I didn't want it to appear that the '.' might be part
of the substituted command.

>
> 4) shell_archive.c
> ereport(lev,
> (errmsg("archive command was terminated by signal %d: %s",
> WTERMSIG(rc), pg_strsignal(WTERMSIG(rc))),
> errdetail("The failed archive command was: %s",
>    xlogarchcmd)));
>

Not done because I didn't want it to appear that the '.' might be part
of the substituted command.

> 5) shell_archive.c
> ereport(lev,
> (errmsg("archive command exited with unrecognized status %d",
> rc),
> errdetail("The failed archive command was: %s",
>    xlogarchcmd)));
>

Not done because I didn't want it to appear that the '.' might be part
of the substituted command.

> 6) matview.c
> ereport(ERROR,
> (errcode(ERRCODE_CARDINALITY_VIOLATION),
> errmsg("new data for materialized view \"%s\" contains duplicate rows
> without any null columns",
> RelationGetRelationName(matviewRel)),
> errdetail("Row: %s",
>    SPI_getvalue(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 1))));
>

Not done because I didn't want it to appear that the '.' might be part
of the substituted row value.

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

  8. Re: Add missing period to DETAIL messages

    Xiaopeng Wang <wxp_728@163.com> — 2026-04-15T05:47:46Z

    在 2026/4/13 9:05, Peter Smith 写道:
> (here's the missing attachment)
>
> On Mon, Apr 13, 2026 at 11:04 AM Peter Smith<smithpb2250@gmail.com> wrote:
>> Found one more oversight.
>>
>> PSA v3.
>>
>> ======
>> Kind Regards,
>> Peter Smith.
>> Fujitsu Australia

Looks good to me.

A small comment is that, the commit message claims only “add missing period to DETAIL messages”, but apparently the patch also changes capitalization in many places, so the commit message should be updated.

Regards,
Xiaopeng Wang

  9. Re: Add missing period to DETAIL messages

    Peter Smith <smithpb2250@gmail.com> — 2026-04-15T07:41:10Z

    On Wed, Apr 15, 2026 at 3:48 PM Xiaopeng Wang <wxp_728@163.com> wrote:
>
...
>
> Looks good to me.
>
> A small comment is that, the commit message claims only “add missing period to DETAIL messages”, but apparently the patch also changes capitalization in many places, so the commit message should be updated.
>

Fair point. Thanks for your review!

PSA v4 which has an improved commit message.

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

  10. Re: Add missing period to DETAIL messages

    Peter Eisentraut <peter@eisentraut.org> — 2026-04-15T18:26:22Z

    On 15.04.26 09:41, Peter Smith wrote:
> On Wed, Apr 15, 2026 at 3:48 PM Xiaopeng Wang <wxp_728@163.com> wrote:
>>
> ...
>>
>> Looks good to me.
>>
>> A small comment is that, the commit message claims only “add missing period to DETAIL messages”, but apparently the patch also changes capitalization in many places, so the commit message should be updated.
>>
> 
> Fair point. Thanks for your review!
> 
> PSA v4 which has an improved commit message.

Most of these look good, but I don't think this is an improvement:

-DETAIL:  syntax error at end of input
+DETAIL:  syntax error at end of input.

The guidelines say that the detail message should be a sentence.  But 
this is not a sentence.  Just adding a period doesn't make it a 
sentence.  IMO, having a period at the end of a thing that is not a 
sentence is worse than not having it be a sentence.  The latter just 
violates our quality standards, the former is confusing for a user who 
sees that particular output.

  11. Re: Add missing period to DETAIL messages

    Peter Smith <smithpb2250@gmail.com> — 2026-04-16T00:22:02Z

    On Thu, Apr 16, 2026 at 4:26 AM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 15.04.26 09:41, Peter Smith wrote:
> > On Wed, Apr 15, 2026 at 3:48 PM Xiaopeng Wang <wxp_728@163.com> wrote:
> >>
> > ...
> >>
> >> Looks good to me.
> >>
> >> A small comment is that, the commit message claims only “add missing period to DETAIL messages”, but apparently the patch also changes capitalization in many places, so the commit message should be updated.
> >>
> >
> > Fair point. Thanks for your review!
> >
> > PSA v4 which has an improved commit message.
>
> Most of these look good, but I don't think this is an improvement:
>
> -DETAIL:  syntax error at end of input
> +DETAIL:  syntax error at end of input.
>
> The guidelines say that the detail message should be a sentence.  But
> this is not a sentence.  Just adding a period doesn't make it a
> sentence.  IMO, having a period at the end of a thing that is not a
> sentence is worse than not having it be a sentence.  The latter just
> violates our quality standards, the former is confusing for a user who
> sees that particular output.

Thanks for your review.

OK, I've removed that "syntax error at end of input." change from the patch.

IIUC, the identical (non-sentence) issue also applies to other ones
like 'syntax error at or near \"%s\".'
So I  removed those from the patch, too.

PSA v5, which makes the above changes.

~~

Now I am having doubts about those '%s depends on column \"%s\".' changes.

Despite looking like sentences, I have no control over the first word
capitalisation, so the results are like:
-DETAIL:  rule _RETURN on view usersview depends on column "seq"
+DETAIL:  rule _RETURN on view usersview depends on column "seq".

In hindsight, maybe just adding periods for these was also not
warranted? What do you think about those?

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

  12. Re: Add missing period to DETAIL messages

    Peter Smith <smithpb2250@gmail.com> — 2026-05-18T05:52:30Z

    This patch seemed to be nearing completion, but then the thread went
quiet for 4 weeks.Is there more I can do to help this patch proceed to
be pushed?

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