Re: pg_dump does not handle indirectly-granted permissions properly

Tom,

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> >> AFAICT, pg_dump has no notion that it needs to be careful about the order
> >> in which permissions are granted. I did
>
> > I'm afraid that's correct, though I believe that's always been the case.
> > I spent some time looking into this today and from what I've gathered so
> > far, there's essentially an implicit (or at least, I couldn't find any
> > explicit reference to it) ordering in ACLs whereby a role which was
> > given a GRANT OPTION always appears in the ACL list before an ACL entry
> > where that role is GRANT'ing that option to another role, and this is
> > what pg_dump was (again, implicitly, it seems) depending on to get this
> > correct in prior versions.
>
> Yeah, I suspected that was what made it work before. I think the ordering
> just comes from the fact that new ACLs are added to the end, and you can't
> add an entry as a non-owner unless there's a grant WGO there already.

Right.

> I suspect it would be easier to modify the backend code that munges ACL
> lists so that it takes care to preserve that property, if we ever find
> there are cases where it does not. It seems plausible to me that
> pg_dump is not the only code that depends on that ordering.

Hm, well, if we're alright with that then I think the attached would
address the pg_dump issue, and I believe this would work as this code is
only run for 9.6+ servers anyway.

This needs more cleanup, testing, and comments explaining why we're
doing this (and then perhaps comments, somewhere.. in the backend ACL
code that explains that the ordering needs to be preserved), but the
basic idea seems sound to me and the case you presented does work with
this patch (for me, at least) whereas what's in master didn't.

Thoughts?

Thanks!

Stephen