| From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
|---|---|
| To: | Zhijie Hou <houzhijie22(at)gmail(dot)com> |
| Cc: | kedar anavardekar <kedar(dot)anavardekar(at)gmail(dot)com>, Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Subject: | Re: Fix WITHOUT OVERLAPS multirange with location replication |
| Date: | 2026-10-12 14:28:55 |
| Message-ID: | CAA4eK1J_kM0HUcSL9J0f0A_Y5vsMjDdaQc5C-qAr+pKjpkTfTg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, Oct 11, 2026 at 8:58 PM Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
>
> 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.
>
About 0001 patch:
The index is created after the conflict detection
+# slot's xmin is pinned, so the sequential scan is used; the locally deleted
+# row 'ABC' must still be reported as update_deleted for the publisher's key
+# 'abc', exactly as if the index had been used.
It is not clear to me why here the behaviour should be similar to
index scan? As index is created after the slot is pinned, so isn't it
okay to behave as there is no index at all? I see the point that for
live tuples scan we may have used index but for conflicts as the index
could be created after the tuple we are searching is deleted, we are
doing sequence scan and comparing just indexed columns and that could
be seen as inconsistent. As this is sort of a grey area and we are not
sure whether to change the code or not, I feel we can fix Paul's
reported issue now as that is a clear bug.
--
With Regards,
Amit Kapila.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Aleksander Alekseev | 2026-10-12 14:49:58 | Re: serializable anomaly - duplicate primary keys |
| Previous Message | ZizhuanLiu X-MAN | 2026-10-12 14:22:37 | Re: examine_variable ignored CollateExpr |