RE: [BUG] Unexpected action when publishing partition tables

From Tues, Sep 7, 2021 12:02 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Mon, Sep 6, 2021 at 1:49 PM houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.com> wrote:
> >
> > I can reproduce this bug.
> >
> > I think the reason is it didn't invalidate all the leaf partitions'
> > relcache when add a partitioned table to the publication, so the
> > publication info was not rebuilt.
> >
> > The following code only invalidate the target table:
> > ---
> > PublicationAddTables
> >         publication_add_relation
> >                 /* Invalidate relcache so that publication info is rebuilt. */
> >                 CacheInvalidateRelcache(targetrel);
> > ---
> >
> > In addition, this problem can happen in both ADD TABLE, DROP TABLE,
> > and SET TABLE cases, so we need to invalidate the leaf partitions'
> > recache in all these cases.
> >
> 
> Few comments:
> =============
>   {
> @@ -664,7 +673,13 @@ PublicationDropTables(Oid pubid, List *rels, bool
> missing_ok)
> 
>   ObjectAddressSet(obj, PublicationRelRelationId, prid);
>   performDeletion(&obj, DROP_CASCADE, 0);
> +
> + relids = GetPubPartitionOptionRelations(relids, PUBLICATION_PART_LEAF,
> + relid);
>   }
> +
> + /* Invalidate relcache so that publication info is rebuilt. */
> + InvalidatePublicationRels(relids);
>  }
> 
> We already register the invalidation for the main table in
> RemovePublicationRelById which is called via performDeletion. I think it is
> better to perform invalidation for partitions at that place.
> Similarly is there a reason for not doing invalidations of partitions in
> publication_add_relation()?

Thanks for the comment. I originally intended to reduce the number of invalid
message when add/drop serval tables while each table has lots of partitions which
could exceed the MAX_RELCACHE_INVAL_MSGS. But that seems a rare case, so ,
I changed the code as suggested.

Attach new version patches which addressed the comment.

Best regards,
Hou zj