Re: Fix apply worker crash when subscriber table has only a deferrable primary key

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix apply worker crash when subscriber table has only a deferrable primary key
Date: 2026-09-29 20:38:21
Message-ID: CAA4eK1KHDofZjMRxJLHzP1Pbnkfekf7RLgR=TUJsxqmrYEKP4w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Sep 29, 2026 at 7:36 AM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> On Tue, Sep 29, 2026 at 3:20 PM Hayato Kuroda (Fujitsu)
> <kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
> >
> > > Thanks for the patch, I've combined your suggested fix and attched
> > > updated patch v2.
> >
> > I checked and no comments for the implementation.
> > Regarding the back patch, the initial issue (FindReplTupleInLocalRel() can cause
> > a crash) should be done till PG17, but second one (RelationFindDeletedTupleInfoSeq()
> > can do a wrong decision) should be done only for PG19/master, right?
>
> Makes sense; I overlooked the fact.
>
> > If so the patch should be separated. Also, a test can be added in 035_conflicts for the
> > second issue.
> >
>
> I’ve attached the separate patches; both apply independently on HEAD
> and their respective backport branches.
>
> I’ve also added a test case in 035_conflicts.pl, for the
> RelationFindDeletedTupleInfoSeq() fix.
>

*
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -593,8 +593,11 @@ RelationFindDeletedTupleInfoSeq(Relation rel,
TupleTableSlot *searchslot,
indexbitmap = RelationGetIndexAttrBitmap(rel,
INDEX_ATTR_BITMAP_IDENTITY_KEY);

- /* fallback to PK if no replica identity */
- if (!indexbitmap)
+ /*
+ * fallback to PK if no replica identity, but only if the PK is not
+ * deferrable.
+ */
+ if (!indexbitmap && OidIsValid(RelationGetPrimaryKeyIndex(rel, false)))
indexbitmap = RelationGetIndexAttrBitmap(rel,

The caller of RelationFindDeletedTupleInfoSeq() already has
information localindexoid/idxisreplident, why can't we use those
values instead of computing the same information again? I am afraid
that computing such an information again could lead to symptoms what
we fixed in the recent commit
ad36e3608c8cb6f0848737ec81e548d4d3a0af3c. I suggest let's first fix
this one and then we can discuss your other patch.

--
With Regards,
Amit Kapila.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Narayanan Venkateswaran 2026-09-29 20:49:24 Re: Proposal: Conflict log history table for Logical Replication
Previous Message Corey Huinker 2026-09-29 20:36:56 Re: Credits For v19