Re: Fix WITHOUT OVERLAPS multirange with location replication

On Tue, Oct 13, 2026 at 5:46 PM Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Oct 13, 2026 at 7:21 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Tue, Oct 13, 2026 at 5:37 AM Paul A Jungwirth
> > <pj(at)illuminatedcomputing(dot)com> 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