On Wed, Mar 24, 2021 at 11:57:37AM -0400, John Naylor wrote: > On Mon, Dec 28, 2020 at 12:32 AM Noah Misch <n...@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 good to 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.gy20...@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.
