Re: Fix WITHOUT OVERLAPS multirange with location replication

Hi,

On Sun, Oct 11, 2026 at 1:51 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Sat, Oct 10, 2026 at 9:31 PM kedar anavardekar
> <kedar(dot)anavardekar(at)gmail(dot)com> wrote:
> >
> > > v2 attached, rebased on current master.
> >
> > Would it make sense to build the recheck bitmap from the scanned
> > index's own key columns (idxrel->rd_index->indkey, up to indnkeyatts)?
> > That would remove the dependency on the relation's current identity.
> > Or do you think the current approach is safe for a reason I'm missing?
> >
>
> IIUC, the current approach is safe because concurrent operations won't
> change index columns. Having said that, I think your suggested
> approach sounds more robust. Additionally, I think we can build the
> bitmap inside (if (eq == NULL)) check to avoid building it again and
> again for each tuple scan.

I think that instead of building a bitmap and passing it to tuples_equal(), we
can write a new helper function that directly traverses the index columns in
the slot and compares each of them, similar to what identity_key_equal() does
in pg_repack. I think that's a more standard approach, because we can use the
index's opclass equality function and collation for the comparison rather than
reusing the table attributes'. It is also more efficient, since it avoids
building a new bitmap and avoids the typecache lookup. It may not show a
performance difference, but I feel it's worth doing better in a common code
path (update/delete replay) like this one.

And based on the above, testing further, I realized this can fix a real issue:
when the index's operator class and collation differ from the table's default
ones, the patch's code (and the code on HEAD) uses the table's pg_attribute
collation to compare the tuple, which is inconsistent with the index's. For the
WITHOUT OVERLAPS index, we need to create a new extension with a new operator
class to reproduce the issue (I did not find a way to use WITHOUT OVERLAPS
together with a index specific collation, otherwise setting a different index
collation can also reproduce it); see 0003 (generated with AI assistance),
which reproduces it that way (it fails with v2 and passes with v3). I don't
think we should commit this heavy test patch, so I'm just sharing it for
reference.

The same issue exists on HEAD as well for the update_deleted detection code
path. I tried writing a patch to replace the update_deleted code path first in
0001 (since it's an existing issue, I've kept it in a separate patch for now to
make it easier to review; we can merge the two patches and commit them together
later if we agree). 0001 includes a test for the collation inconsistency issue.

And I rebased the WITHOUT OVERLAPS fix on top of it, modifying it to use the
new approach for tuple comparison and slightly adjusting some commit messages
and comments.

I'm sharing both patches here since both touch the same code path
and overlap. Hope it helps move the thread forward.

Best Regards,
Zhijie Hou