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

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: StringInfo fixes, v19 edition. Plus a few oddities
Date: 2026-04-24 06:59:42
Message-ID: CAHut+PvY2xJyONaTqV3qZoT1vsXNVOmQHoG6+hC7Not7KyZ9nQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Vignesh.

Some review comments for patch v2-0001.

======

append_tuple_value_detail:

1.
+ if (first)
+ appendStringInfoString(buf, "tuple data not available (insufficient
privileges)");

The logic to use first==true to mean "insufficient privilege" seems
strange. Presumably, the only way to get this message is when the
`continue` of the prior loop happened at *every* iteration.

Isn't it ambiguous? e.g. What if there are multiple tuple_values but
only 1 tuple_value was NULL? Then the boolean `first` will not be
true, so there was something unavailable due to insufficient
privileges that has gone unreported (???).

~~~

errdetail_apply_conflict:

2.
case CT_UPDATE_DELETED:
- appendStringInfoString(&err_detail, _("Could not find the row to be
updated"));

- append_tuple_value_detail(&err_detail,
- list_make2(remote_desc, search_desc),
- true);
+ append_tuple_value_detail(&tuple_buf,
+ list_make2(remote_desc, search_desc));
+ appendStringInfo(&err_detail, _("Could not find the row to be
updated: %s.\n"),
+ tuple_buf.data);

Results in a whitespace line after the CT_UPDATE_DELETED: that was not
there before.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message shveta malik 2026-04-24 07:17:03 Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE
Previous Message Pavel Stehule 2026-04-24 06:56:36 Re: PoC - psql - emphases line with table name in verbose output