On Friday, November 12, 2021 8:15 AM Euler Taveira <euler@eulerto.com> wrote:
> I reviewed your patch and I think the fix could be simplified by
>
> if (OidIsValid(indexOid))
> CacheInvalidateRelcache(rel);
>
> If indexOid is valid it is a REPLICA IDENTITY INDEX. A few lines above there is
> a check for a valid indexOid that makes sure the index is already marked as a
> replica identity; if so, it bail out. If it is not, the relation should be
> invalidated. Am I missing something?
Thanks for reviewing !
But I am not sure it's better to simplify the code like
"if (OidIsValid(indexOid)) CacheInvalidate". After this change, it will
also invalidate the relcache when changing REPLICA IDENTITY from
DEFAULT/FULL/NOTHING to USING INDEX which I think is unnecessary, because the
invalidate for these cases was already handled by
"CatalogTupleUpdate(pg_class".
So, I still think the flag need_rel_inval is needed.
> I also modified your test case to include a DELETE command, wait the initial
> table sync to avoid failing a subsequent test and improve some comments.
Thanks!
Attach the patch which changed the code use need_rel_inval flag.
Also ran pgperltidy for the testcases. I will post patches for backbranch
if there are no more comments.
Best regards,
Hou zj
