Re: Fix for OWNER TO breaking ACLs

Christopher Kings-Lynne <chriskl(at)familyhealth(dot)com(dot)au> writes:
> Attached is a patch that fixes the owner change command on objects that
> have privileges.

Applied with revisions. Just FYI ---

* The aclnewowner code wasn't really right at all. It was changing
ai_grantee without checking whether that value was a UID, GID, or WORLD;
and it was not adequately handling the problem of merging duplicate
entries afterwards. (You have to think about entries with the new owner
as grantor as well as such entries with grantee; and even considering
only the grantee case, it's wrong to suppose there can be only one.)
I replaced the logic with a general-purpose search for duplicate
entries, which might be overkill but it will reliably get the right
answer.

* You had consistently changed the simple_heap_update calls to do the
wrong thing. (I'm surprised it didn't blow up on you in your testing.)
In a sequence like

newtuple = heap_modifytuple(tup, rel, repl_val, repl_null, repl_repl);

simple_heap_update(rel, &newtuple->t_self, newtuple);
CatalogUpdateIndexes(rel, newtuple);

the second parameter to simple_heap_update *must* be newtuple->t_self
not tup->t_self. The reason is that simple_heap_update stores the new
physical location of the updated tuple back into that parameter, and
then the CatalogUpdateIndexes call relies on newtuple->t_self to
generate new index entries. The way you had it coded, it was generating
new index entries pointing at the old version of the tuple ...

regards, tom lane