Add missing period to DETAIL messages
Hi,
According to the error message style guide [1]https://www.postgresql.org/docs/devel/error-style-guide.html#ERROR-STYLE-GUIDE-GRAMMAR-PUNCTUATION "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
Attachments:
v1-0001-Add-missing-period-to-DETAIL-messages.patchapplication/octet-stream; name=v1-0001-Add-missing-period-to-DETAIL-messages.patchDownload+67-67
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-PUNCTUATIONKind 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/
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
Attachments:
v2-0001-Add-missing-period-to-DETAIL-messages.patchapplication/octet-stream; name=v2-0001-Add-missing-period-to-DETAIL-messages.patchDownload+73-73
Found one more oversight.
PSA v3.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
(here's the missing attachment)
Show quoted text
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
Attachments:
v3-0001-Add-missing-period-to-DETAIL-messages.patchapplication/octet-stream; name=v3-0001-Add-missing-period-to-DETAIL-messages.patchDownload+74-74
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
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
在 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
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
Attachments:
v4-0001-DETAIL-messages-fix-capitalization-and-add-missin.patchapplication/octet-stream; name=v4-0001-DETAIL-messages-fix-capitalization-and-add-missin.patchDownload+74-74
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.
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