Re: Alter all tables in schema owner fix

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