Re: BUG #17062: Assert failed in RemoveRoleFromObjectPolicy() on DROP OWNED policy applied to duplicate role

On Thu, Jun 17, 2021 at 05:51:18PM -0400, Tom Lane wrote:
> While I'm whining ... that function's permissions checks seem
> completely out of line too. How is it that, if I have the right
> to drop some role, I lose that right if the role is mentioned in
> a policy of some relation I don't own? It feels like this function
> was written by copy-and-pasting a whole bunch of irrelevant logic.

Hm. Wouldn't it be better to do something similar to 21378e1f where
we ensure that there are no duplicated role OIDs in the catalogs to
begin with, letting the drop code as it is now? The array build
happens in policy_role_list_to_array() for CREATE/ALTER POLICY, so
that's more than just a CCI. And I think that making the DROP OWNED
BY code more reliable with drops of the same role makes the handling
of shdepend entries a bit shaky. Back-branches would need to handle a
multi-role logic, now nobody has complained either about this bug
issue for years.

> * Locking. It acquires lock on the policy's relation only after
> it looks up the pg_policy entry. By that point the entry could
> be gone or modified.

Yeah. The policy could be gone, or its relations, so it is possible
to hit two elog() here. rename_policy() does not take a lock on the
policy's relation for one.

> * Why is it expensively reconstructing the dependencies of the
> policy expressions? Those aren't going to be changed by this
> operation. AFAICS it ought to be sufficient to remove and
> rebuild the policy's shared dependencies.

For the shdepend entries dropped and then recreated from scratch, I
agree that this is a waste. That would be simple enough to fix if we
make shdepDropDependency() available, and simpler to call it in loop
with the roles dropped.
--
Michael