Re: Alter all tables in schema owner fix

On 12/2/21, 7:07 PM, "vignesh C" <vignesh21@gmail.com> wrote:
> Currently while changing the owner of ALL TABLES IN SCHEMA
> publication, it is not checked if the new owner has superuser
> permission or not. Added a check to throw an error if the new owner
> does not have superuser permission.
> Attached patch has the changes for the same. Thoughts?

Yeah, the documentation clearly states that "the new owner of a FOR
ALL TABLES or FOR ALL TABLES IN SCHEMA publication must be a
superuser" [0].

+/*
+ * Check if any schema is associated with the publication.
+ */
+static bool
+CheckSchemaPublication(Oid pubid)

I don't think the name CheckSchemaPublication() accurately describes
what this function is doing.  I would suggest something like
PublicationHasSchema() or PublicationContainsSchema().  Also, much of
this new function appears to be copied from GetPublicationSchemas().
Should we just use that instead?

+CREATE ROLE regress_publication_user3 LOGIN SUPERUSER;
+GRANT regress_publication_user2 TO regress_publication_user3;
+SET ROLE regress_publication_user3;
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub4 FOR ALL TABLES IN SCHEMA pub_test;
+RESET client_min_messages;
+SET ROLE regress_publication_user;
+ALTER ROLE regress_publication_user3 NOSUPERUSER;
+SET ROLE regress_publication_user3;

I think this test setup can be simplified a bit:

    CREATE ROLE regress_publication_user3 LOGIN;
    GRANT regress_publication_user2 TO regress_publication_user3;
    SET client_min_messages = 'ERROR';
    CREATE PUBLICATION testpub4 FOR ALL TABLES IN SCHEMA pub_test;
    RESET client_min_messages;
    ALTER PUBLICATION testpub4 OWNER TO regress_publication_user3;
    SET ROLE regress_publication_user3;

Nathan

[0] https://www.postgresql.org/docs/devel/sql-alterpublication.html