On Thu, Mar 6, 2025 at 5:16 PM Hayato Kuroda (Fujitsu) <kuroda.hay...@fujitsu.com> wrote: > > Dear Amit, hackers, > > > Yeah, this is a good improvement and the patch looks safe to me, so I > > pushed with minor changes in the comments. > > Thanks! PSA rebased patch. >
Few comments: 1. Why do we need to invalidate relsync entries when owner of its publication changes? I think the owner change will impact the future Alter Publication ... Add/Drop/Set/Rename operations as that will be allowed only to new owner (or super users), otherwise, there shouldn't be an impact on RelSyncCache entries. Am, I missing something? 2. + if (pubform->puballtables) + { + CacheInvalidateRelSyncAll(); + } + else + { + List *relids = NIL; + List *schemarelids = NIL; + + /* + * For partition table, when we insert data, get_rel_sync_entry is + * called and a hash entry is created for the corresponding leaf table. + * So invalidating the leaf nodes would be sufficient here. + */ + relids = GetPublicationRelations(pubform->oid, + PUBLICATION_PART_LEAF); + schemarelids = GetAllSchemaPublicationRelations(pubform->oid, + PUBLICATION_PART_LEAF); + + relids = list_concat_unique_oid(relids, schemarelids); + + InvalidateRelSyncCaches(relids); + } + + CatalogTupleUpdate(rel, &tup->t_self, tup); Shouldn't we need to update the CatalogTuple before invalidations. 3. + if (pubform->puballtables) + { + CacheInvalidateRelSyncAll(); + } + else + { + List *relids = NIL; + List *schemarelids = NIL; + + /* + * For partition table, when we insert data, get_rel_sync_entry is + * called and a hash entry is created for the corresponding leaf table. + * So invalidating the leaf nodes would be sufficient here. + */ + relids = GetPublicationRelations(pubform->oid, + PUBLICATION_PART_LEAF); + schemarelids = GetAllSchemaPublicationRelations(pubform->oid, + PUBLICATION_PART_LEAF); + + relids = list_concat_unique_oid(relids, schemarelids); + + InvalidateRelSyncCaches(relids); + } This is a duplicate code. Can we write a function to eliminate this duplicacy? -- With Regards, Amit Kapila.