All,
We, as a community, have gotten flak from time-to-time about the superuser role. We also tend to wish to avoid
unnecessaryoptimization as it complicates the code base and makes folks reviewing the code wonder at the exceptions.
As such, I wonder at a few of superuser() checks we have today which appear to be entirely superfluous, specifically:
replication/logical/logicalfuncs.c check_permissions()
My 2c about this function is that it should be completely removed and the place where it's checked replaced
withjust the 'has_rolreplication' call and error. It's only called in one place and it'd be a simple one-liner
anyway. As for has_rolreplication, I don't understand why it's in miscinit.c when the rest of the has_* set is
inacl.c.
replication/slotfuncs.c - more or less the same
commands/alter.c AlterObjectOwner_internal() There's a shortcut here for superuser() that appears entirely
redundantas the immediately following 'has_privs_of_role()' will return true for all superuser, as will the later
check_is_member_of_role() call, and the pg_namespace_aclcheck will also return true. Perhaps I'm missing
something,but why isn't this superuser() check completely redundant and possible not ideal (what if Anum_name
isvalid but NULL after all..?).
commands/tablecmds.c ATExecChangeOwner() The superuser check here looks to just be avoiding extra permission
checks,but that could change and we might eventually end up in a situation similar to above where other checks are
happening (possibly to avoid a crash) but don't end up happenning for superuser by mistake. I don't feel like
tableowner changes happen so often that we need to avoid a couple extra function calls and so I would recommend
rippingout the explicit superuser() check here.
commands/typecmds.c AlterTypeOwner() More-or-less the same as above.
commands/foreigncmds.c AlterForeignServerOwner_internal() Ditto.
Removing these design patterns may also help to avoid ending up with more of them in the future as folks copy and/or
criboff of what we've already done to implement their features...
Thanks!
Stephen
