On Fri, Jan 21, 2022 at 3:12 PM Kenaniah Cerny <kenaniah@gmail.com> wrote:
The latest rebased version of the patch is attached.
As I was just reminded, we tend to avoid specifying specific PostgreSQL versions in our documentation. We just say what the current version does. Here, the note sentences at lines 62 and 63 don't follow documentation norms on that score and should just be removed. The last two sentences belong in the main description body, not a note. Thus the whole note goes away.
I don't think I really appreciated the value this feature would have when combined with the predefined roles like pg_read_all_data and pg_write_all_data.
I suppose I don't really appreciate the warning about SUPERUSER, etc...or at least why this warning is somehow specific to the per-database version of role membership. If this warning is desirable it should be worded to apply to role membership in general - and possibly proposed as a separate patch for consideration.
I didn't dive deeply but I think we now have at three places in the acl.c code where after setting memlist from the system cache we perform nearly identical for loops to generate the final roles_list. Possibly this needs a refactor first so that you can introduce the per-database stuff more succinctly. Basically, the vast majority of this commit is just adding InvalidOid and databaseOid all other the place - with a few minor code changes to accommodate the new arguments. The acl.c code should try and be made done the same after post-refactor.