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

Peter Smith <smithpb2250@gmail.com>

From: Peter Smith <smithpb2250@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Amit Kapila <amit.kapila16@gmail.com>, David Rowley <dgrowleyml@gmail.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2026-04-24T06:59:42Z
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



Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Simplify translatable messages for tuple value details in conflict.c.

  2. Improve various new-to-v19 appendStringInfo calls