On Tue, Oct 13, 2026 at 5:46 PM Zhijie Hou <[email protected]> wrote: > > Hi, > > On Tue, Oct 13, 2026 at 7:21 PM Amit Kapila <[email protected]> wrote: > > > > On Tue, Oct 13, 2026 at 5:37 AM Paul A Jungwirth > > <[email protected]> wrote: > > > > > > This v4 looks good to me. Thank you for helping with this! Maybe one > > > minor change: > > > > > > src/test/subscription/t/035_conflicts.pl > > > +# Publisher table holding the row that will be updated. > > > +$node_B->safe_psql( > > > + 'postgres', " > > > + CREATE TABLE tab_mr (id int4multirange, valid_at daterange, v int); > > > + ALTER TABLE tab_mr REPLICA IDENTITY FULL; > > > + INSERT INTO tab_mr VALUES ('{[1,5)}', '[2000-01-01,2010-01-01)', 1); > > > + ALTER PUBLICATION tap_pub_B ADD TABLE tab_mr; > > > +"); > > > > > > If we say PRIMARY KEY (id, valid_at WITHOUT OVERLAPS) then we don't > > > need REPLICA IDENTITY FULL. > > > > > > > Yeah, but keeping RI FULL also doesn't harm; rather it tests something > > extra which is that it can extract required values from the full > > tuple. So, I am okay either way. I have modified the comments at > > multiple places in the patch as I find there is a lot of redundancy in > > the comments. See attached top-up patch. BTW, we need a separate patch > > for v18 as well. > > Thanks for the diff. I have merged and prepared the patches for > back branches as well. >
Thanks for updated patches. The fix looks good to me. A few comments: 1) v5_PG18-0001: the commit message mentions RelationFindDeletedTupleInfoByIndex() and the update_deleted vs update_missing case, but it doesn't apply to PG18. Maybe that part could be dropped. A couple of nits, please consider if they make sense: 2) The updated header comment of RelationFindReplTupleByIndex() reads a bit confusing to me, how about something like: * 'skipduplicates' specifies whether the first matching tuple can be used * without comparing it against 'searchslot', unless the index scan sets * xs_recheck (as with a lossy GiST index for WITHOUT OVERLAPS), in which case * only the index key columns are compared, using the index's own equality * semantics. If false, all matching tuples are compared against 'searchslot', * which must contain a complete row. 3) 035_conflicts.pl: +ok( $node_A->poll_query_until( + 'postgres', + "SELECT xmin::text = '$pinned_xid' FROM pg_replication_slots WHERE slot_name 'pg_conflict_detection'" + ), Should we use "xmin::text::bigint = $pinned_xid" here, to match the other slot xmin checks in the same file? -- Thanks, Nisha
