Re: Alter all tables in schema owner fix - Mailing list pgsql-hackers

From Greg Nancarrow
Subject Re: Alter all tables in schema owner fix
Date
Msg-id CAJcOf-f1Q7JotqpymCgG_+jFw+6cGao6C=34xZ8EGbaTr2pB+A@mail.gmail.com
Whole thread Raw
In response to Alter all tables in schema owner fix  (vignesh C <vignesh21@gmail.com>)
Responses Re: Alter all tables in schema owner fix  ("Bossart, Nathan" <bossartn@amazon.com>)
Re: Alter all tables in schema owner fix  (vignesh C <vignesh21@gmail.com>)
List pgsql-hackers
On Fri, Dec 3, 2021 at 2:06 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?
>

It looks OK to me, but just two things:

1) Isn't it better to name "CheckSchemaPublication" as
"IsSchemaPublication", since it has a boolean return and also
typically CheckXXX type functions normally do checking and error-out
if they find a problem.

2) Since superuser_arg() caches previous input arg (last_roleid) and
has a fast-exit, and has been called immediately before for the FOR
ALL TABLES case, it would be better to write:

+ if (CheckSchemaPublication(form->oid) && !superuser_arg(newOwnerId))

as:

+ if (!superuser_arg(newOwnerId) && IsSchemaPublication(form->oid))


Regards,
Greg Nancarrow
Fujitsu Australia



pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: pg_get_publication_tables() output duplicate relid
Next
From: "Bossart, Nathan"
Date:
Subject: Re: Alter all tables in schema owner fix