On 12/2/21, 7:07 PM, "vignesh C" <vignes...@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