RE: [BUG] Unexpected action when publishing partition tables

From Tues, Sep 7, 2021 12:02 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Mon, Sep 6, 2021 at 1:49 PM houzj(dot)fnst(at)fujitsu(dot)com <houzj(dot)fnst(at)fujitsu(dot)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