| 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
| 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) |