On Fri, Aug 14, 2026 at 10:09 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> 0001: heap_update check
>
Few comments on 0001:
===================
1.
@@ -3453,6 +3478,34 @@ heap_update(Relation relation, const
ItemPointerData *otid, HeapTuple newtup,
id_attrs, &oldtup,
newtup, &id_has_external);
+ id_changed = bms_overlap(modified_attrs, id_attrs);
+
+ /*
+ * If the update could be transformed into an insert by a publication row
+ * filter during decoding, reject it when it would lose an unchanged
+ * out-of-line value of a column that is not part of the replica identity.
+ */
+ if (check_unchanged_external && id_changed)
Why did you place the above check in heap_update before label l2? If
the check ran before l2: (e.g. right where
modified_attrs/id_key_changed are first computed), a raised
ereport(ERROR) there could fire for an update attempt that was never
actually going to happen, the row might get updated by someone else in
the interim, EvalPlanQual retries with a different row version, and
our error would have been wrong or at least premature. Placing the
check after the TM_Ok confirmation and after the VM-pin retry (i.e.
after every goto l2 site) guarantees no more retries follow, so
raising the error here means the update really was about to proceed
against this exact tuple.
2. Can we check the required value from relation's pubdesc before
calling RelationBuildPublicationDesc()?
> I haven't added doc yet, but I can add it once we reach consensus.
>
Feel free to add where required.
--
With Regards,
Amit Kapila.
