On Fri, Dec 03, 2021 at 05:20:35PM +0000, Bossart, Nathan wrote: > On 12/2/21, 11:57 PM, "tanghy.f...@fujitsu.com" <tanghy.f...@fujitsu.com> > wrote: > > Thanks for your patch. > > I tested it and it fixed this problem as expected. It also passed "make > > check-world". > > +1, the patch looks good to me, too. My only other suggestion would > be to move IsSchemaPublication() to pg_publication.c
There is more to that, no? It seems to me that anything that opens PublicationNamespaceRelationId should be in pg_publication.c, so that would include RemovePublicationSchemaById(). If you do that, GetSchemaPublicationRelations() could be local to pg_publication.c. + tup = systable_getnext(scan); + if (HeapTupleIsValid(tup)) + result = true; This can be written as just "result = HeapTupleIsValid(tup)". Anyway, this code also means that once we drop the schema this publication won't be considered anymore as a schema publication, meaning that it also makes this code weaker to actual cache lookup failures? I find the semantics around pg_publication_namespace is bit weird because of that, and inconsistent with the existing puballtables/pg_publication_rel. -- Michael
signature.asc
Description: PGP signature
