From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com> |
Subject: | Re: [BUG]Invalidate relcache when setting REPLICA IDENTITY |
Date: | 2021-11-11 04:29:15 |
Message-ID: | CAA4eK1Ki-kUx52uRER3bZSC6bLc=iix+QnBxs1pZSHHYZOEF1A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Nov 11, 2021 at 9:37 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Nov 11, 2021 at 7:07 AM houzj(dot)fnst(at)fujitsu(dot)com
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > On Wed, Nov 10, 2021 7:29 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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.".
>
Few comments on testcase added by the patch:
==================================
1.
+$node_subscriber->safe_psql('postgres',
+);
It is not clear what is the purpose served by this statement.
2.
+# replication of the table without replica identity index but not primary key
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE tab_replidentity_index(a int not null, b int not null)");
I think the part of the comment "but not primary key" is not required.
3. Can we move this test to subscription/t/100_bugs?
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Dian M Fay | 2021-11-11 04:58:44 | Re: [PATCH] postgres_fdw: suppress explicit casts in text:text comparisons (was: column option to override foreign types) |
Previous Message | Amit Kapila | 2021-11-11 04:07:32 | Re: [BUG]Invalidate relcache when setting REPLICA IDENTITY |