Re: Add missing period to DETAIL messages

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add missing period to DETAIL messages
Date: 2026-04-14 06:28:37
Message-ID: CAHut+PvmHoOonAosaO1YUnU7fa+4BpUZGA4oHq7pPF7_=+FrEQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Apr 14, 2026 at 3:51 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Mon, 13 Apr 2026 at 06:36, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > (here's the missing attachment)
> >
> > On Mon, Apr 13, 2026 at 11:04 AM Peter Smith <smithpb2250(at)gmail(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nisha Moond 2026-04-14 06:30:12 Support EXCEPT for TABLES IN SCHEMA publications
Previous Message Chao Li 2026-04-14 06:28:24 Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)