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

On Mon, Oct 5, 2026 at 1:08 PM Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Mon, Oct 5, 2026 at 12:30 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >
> > On Sat, Oct 3, 2026 at 2:15 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Thu, Oct 1, 2026 at 11:33 PM Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
> > > >
> > > > I merged a deferrable key test from 0002 into 0001 in this version.
> > > >
> > > > Here is the V7 patch that addressed above comments. I confirmed it applies
> > > > cleanly on all required branches and the test passed.
> > > >
> > >
> > > Thanks, I've pushed the patch.
> > >
> >
> > Thanks for pushing the patch.
> > Here is the rebased v4 patch for the remaining issue in this thread.
>
> I confirmed the patch fixes the issue.
>
> I initially had a concern while reviewing the code: there might be a risk that
> the index fetched via the RelationGetxxx() function is inconsistent with the
> computed and cached localindexoid. After testing, I don't think this can
> happen, because no invalidations can be processed between
> FindLogicalRepLocalIndex() and logicalrep_rel_mark_updatable() so even if the
> index is dropped concurrently in between, it won't cause real issues.
>

Yes, agree the race is not possible here.

> That said, even if there's no race here, would it be better to simply use the
> computed localindexoid for the updatable check rather than fetching it from
> relcache again? I think that would make the code simpler and safer. I'm sharing
> a small top-up patch for reference.
>

This is the same idea Kuroda-san suggested at [1]. I’ve reviewed and
tested it, and it works well.
Attached is the updated patch. The code is the same as your top-up
patch, with these small changes:
- Added a comment above logicalrep_rel_mark_updatable() that the
caller must set localindexoid and idxisreplident first, since the
function now depends on that ordering.
- Kept the comment for the publisher table RI-FULL check

I’ve also updated the commit message to describe the new approach.

v5-001 applies cleanly on PG19 and PG18, but not on PG17. Attached a
separate PG17 patch.

[1] https://www.postgresql.org/message-id/OS7PR01MB183175A0C8C0710245FE1E135F5962%40OS7PR01MB18317.jpnprd01.prod.outlook.com

--
Thanks,
Nisha