On Wed, Mar 24, 2021 at 11:57:37AM -0400, John Naylor wrote:
> On Mon, Dec 28, 2020 at 12:32 AM Noah Misch <noah@leadboat.com> wrote:
> > [v2]
>
> Hi Noah,
>
> In the refactoring patch, there is a lingering comment reference to roles_has_privs_of(). Aside from that, it looks
goodto me. A possible thing to consider is an assert that is_admin is not null where we expect that.
Thanks. The next version will contain the comment fix and
"Assert(OidIsValid(admin_of) == PointerIsValid(is_admin));".
> The database owner role patch looks good as well.
Do you plan to put the CF entry into Ready for Committer, or should the
patches wait for another review?
> > I ended up blocking DDL that creates role memberships involving the new role;
> > see reasons in user.c comments. Lifting those restrictions looked feasible,
> > but it was inessential to the mission, and avoiding unintended consequences
> > would have been tricky. Views "information_schema.enabled_roles" and
> > "information_schema.applicable_roles" list any implicit membership in
> > pg_database_owner, but pg_catalog.pg_group and psql \dgS do not. If this
> > leads any reviewer to look closely at applicable_roles, note that its behavior
> > doesn't match its documentation
> > (https://postgr.es/m/flat/20060728170615.GY20016@kenobi.snowman.net).
>
> Is this something that needs fixing separately?
It is bug, but I think fixing it is not very urgent and should happen
separately, if at all.
