On Mon, Oct 5, 2026 at 1:08 PM Zhijie Hou <[email protected]> wrote: > > Hi, > > On Mon, Oct 5, 2026 at 12:30 PM Nisha Moond <[email protected]> wrote: > > > > On Sat, Oct 3, 2026 at 2:15 AM Amit Kapila <[email protected]> wrote: > > > > > > On Thu, Oct 1, 2026 at 11:33 PM Zhijie Hou <[email protected]> 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
v5-0001-Don-t-treat-a-deferrable-PK-as-replica-identity-o.patch
Description: Binary data
v5_pg17-0001-Don-t-treat-a-deferrable-PK-as-replica-ident.patch
Description: Binary data
