RE: [BUG]Invalidate relcache when setting REPLICA IDENTITY - Mailing list pgsql-hackers

From houzj.fnst@fujitsu.com
Subject RE: [BUG]Invalidate relcache when setting REPLICA IDENTITY
Date
Msg-id OS0PR01MB5716714BC4EA96EB207C768B94949@OS0PR01MB5716.jpnprd01.prod.outlook.com
Whole thread Raw
In response to RE: [BUG]Invalidate relcache when setting REPLICA IDENTITY  ("houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>)
Responses Re: [BUG]Invalidate relcache when setting REPLICA IDENTITY
List pgsql-hackers
On Thursday, November 11, 2021 5:36 PM houzj.fnst@fujitsu.com wrote:
> On Thur, Nov 11, 2021 12:08 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Thu, Nov 11, 2021 at 7:07 AM houzj.fnst@fujitsu.com
> <houzj.fnst@fujitsu.com> wrote:
> > >
> > > On Wed, Nov 10, 2021 7:29 PM Amit Kapila <amit.kapila16@gmail.com>
> wrote:
> > > >
> > > > I don't understand the purpose of idx_b in the above test case,
> > > > why is it required to reproduce the problem?
> > > > @@ -15488,6 +15488,7 @@ relation_mark_replica_identity(Relation
> > > > rel,
> > char
> > > > ri_type, Oid indexOid,
> > > >   CatalogTupleUpdate(pg_index, &pg_index_tuple->t_self,
> > pg_index_tuple);
> > > >   InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
> > > >   InvalidOid, is_internal);
> > > > + CacheInvalidateRelcache(rel);
> > > >
> > > > CatalogTupleUpdate internally calls heap_update which calls
> > > > CacheInvalidateHeapTuple(), why is that not sufficient for invalidation?
> > >
> > > I think it's because the bug happens only when changing REPLICA
> > > IDENTITY
> > index
> > > from one(idx_a) to another one(idx_b).
> > >
> >
> > Okay, but do we need to invalidate the rel cache each time the dirty
> > flag is set? Can't we do it once outside the foreach index loop? I
> > think we can record whether to do relcache invalidation in a new
> > boolean variable need_rel_inval or something like that. Also, it is
> > better if you can add a comment on the lines of: "Invalidate the
> > relcache for the table so that after we commit all sessions will
> > refresh the table's replica identity index before attempting any
> > update on the table.".
> 
> I agree.
> 
> Attach the new version fix patch which move the invalidation out of the loop and
> addressed the comments[1] for the testcases.

Also attach the patches for back branch and remove some unnecessary
changes from pgindent.

Best regards,
Hou zj

Attachment

pgsql-hackers by date:

Previous
From: Shay Rojansky
Date:
Subject: Re: Should AT TIME ZONE be volatile?
Next
From: Andrey Borodin
Date:
Subject: Re: 2021-11-11 release announcement draft