Re: StringInfo fixes, v19 edition. Plus a few oddities

On Thu, 23 Apr 2026 at 16:07, Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Thursday, April 23, 2026 12:03 AM vignesh C <vignesh21@gmail.com> wrote:
> > On Sun, 12 Apr 2026 at 06:42, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >
> > > David Rowley <dgrowleyml@gmail.com> writes:
> > > > append_tuple_value_detail() contains some pretty weird translation
> > > > strings. The code currently does:
> > >
> > > > /* translator: This is the terminator of a conflict message */
> > > > appendStringInfoString(buf, _("."));
> > >
> > > > I'm not a translator, but isn't that going to be pretty hard to know
> > > > what to do with, given that the same string could be used for
> > > > anything else in the code and need something completely different
> > > > done? That same function also contains a _(": ") and _(", ") which
> > > > seem equally hard to deal with.
> > >
> > > Yeah, this seems to me to fly in the face of our style guide's rule
> > > about "don't construct messages out of parts".  And the reason for the
> > > rule is precisely that stuff like this poses insoluble problems for
> > > translators.
> > >
> > > I think it'd be reasonable to make this helper function build a string
> > > like
> > >         (value, value, value)
> > > with the punctuation being untranslated on the grounds that this is
> > > standard SQL notation.  Then that could be plugged into a translatable
> > > message that might hopefully represent a full sentence with a %s for
> > > the tuple data.
> >
> > I've updated the code to avoid constructing messages from translated
> > fragments. append_tuple_value_detail() now only builds a SQL-style tuple
> > string like (v1, v2, v3) without any translation or punctuation.
> > All punctuation and sentence construction are moved to the callers, which
> > now use a single translatable string with a %s placeholder for the tuple data.
> > Attached patch has the changes for the same.
>
> The patch seems to change the message content by adding parentheses around the
> tuple detail section[1]. However, IIUC, we only need parentheses around the value,
> not around the keyword.
>
> [1]
> before: ... row to be deleted: replica identity (a)=(10)
> After :  ... row to be deleted: (replica identity (a)=(10))

I checked this path and confirmed that get_tuple_desc() already wraps
the values in parentheses, so adding another layer here would be
redundant.
Additionally, I have introduced a fallback message "tuple data not
available (insufficient privileges)" to handle cases where the tuple
is not available due to insufficient privileges.

The attached v2 version patch has the changes for the same.

Regards,
Vignesh