| From: | Nikhil Sontakke <nikhil(at)planetscale(dot)com> |
|---|---|
| To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Subject: | Stale comments for the tuple_update/tuple_delete options parameter |
| Date: | 2026-08-11 10:12:46 |
| Message-ID: | CA+UBoq21SzkjThYMSwSpVU-jwzpuEsf22Hq1FPbcKcDc43C29g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
While reading the table AM interface I noticed that the comments for the
options parameter of tuple_update() and tuple_delete() have drifted from
the code. I was planning on using this options parameter myself and got
confused by the little inconsistency initially.
db89a47115f added the parameter, documented it as recognizing no values,
and marked heap_update()'s copy pg_attribute_unused(). All of that was
accurate at the time. 28d534e2ae0 then added TABLE_UPDATE_NO_LOGICAL
and TABLE_DELETE_NO_LOGICAL and began reading the parameter, but the
earlier statements were left in place.
table_tuple_update() consequently documents options twice:
* options - bitmask of options. No values are currently recognized.
* crosscheck - if not InvalidSnapshot, also check old tuple against this
* options - These allow the caller to specify options that may change the
* behavior of the AM. The AM will ignore options that it does not support.
* TABLE_UPDATE_NO_LOGICAL -- force-disables the emitting of logical
* decoding information for the tuple.
The first copy contradicts the second, and the description of crosscheck
is stranded between them.
table_tuple_delete() has a related omission: it lists only
TABLE_DELETE_CHANGING_PARTITION, so TABLE_DELETE_NO_LOGICAL is
undocumented at the interface an out-of-tree AM author would read, even
though heap_delete() has acted on it since the same commit.
The attached patch drops the stale copy, restores the parameters to
signature order, and adds the missing delete flag. Both descriptions
now follow the wording already used for TABLE_INSERT_NO_LOGICAL, which
also explains why the flag exists -- table rewrites where
RelationIsLogicallyLogged() is not yet accurate for the new relation --
since repack.c passes all three for that same reason.
It also removes the pg_attribute_unused() marker from heap_update(),
which has read options since 28d534e2ae0.
Comments only; no behaviour change.
Thanks,
Nikhil
---
Nikhil Sontakke
PlanetScale Postgres Core Team
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-Fix-stale-comments-for-the-table-AM-update-and-delet.patch | application/octet-stream | 3.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2026-08-11 10:13:22 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Heikki Linnakangas | 2026-08-11 09:41:49 | Re: Crash issue in PG18.5 regression |