| 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.
| 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 |