Interesting near-bug in shared-dependency management

I am fooling around with deleting pg_default_acl entries when they are
changed to match the default value, as per yesterday's discussion:
http://archives.postgresql.org/pgsql-hackers/2010-04/msg00114.php

I noticed what seemed to be a bug in SetDefaultACL(): the list of "old
ACL members" that it generates to pass to updateAclDependencies() is
extracted from old_acl, even if that value is generated by acldefault()
rather than having been read from the table. That means that any roles
mentioned in the default ACL will not be added as dependencies during an
insertion of a new pg_default_acl entry, because updateAclDependencies
will think they're already listed as dependencies. I said to myself,
"this is stupid: obviously the list of old members should be empty
during an insertion". So I changed it, and promptly got an Assert
failure in the regression tests.

On investigation, the reason for the Assert failure is that
updateAclDependencies supposes that the difference between the old and
new members list can consist only of additions during a GRANT, and only
of removals during a REVOKE. The failing test case is a REVOKE that is
revoking some of the non-empty default privileges for functions, and so
the result ACL isn't empty. If we tell updateAclDependencies that the
starting ACL was empty, it fails.

Now upon reflection, there is no observable fault here, because in all
cases the hard-wired default ACL for an object type mentions only the
owning role, and updateAclDependencies ignores that role anyway. So
passing it an initial membership list containing only the owning role
doesn't create a problem, even though that list isn't an accurate
reflection of the prior state of pg_default_acl. Still, it seems like
trouble waiting to happen.

I am very strongly tempted to make this more robust by recoding
updateAclDependencies to not assume that additions are only possible in
GRANT and removals only possible in REVOKE, but just have it examine the
given membership lists and add or delete dependency entries according to
the observed differences in the lists. It probably doesn't even need to
have isGrant as an argument.

Comments?

regards, tom lane