On Fri, Dec 03, 2021 at 05:20:35PM +0000, Bossart, Nathan wrote:
> On 12/2/21, 11:57 PM, "tanghy.fnst@fujitsu.com" <tanghy.fnst@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