Re: pg_dump broken for non-super user

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> Why is it that we need to lock a table at all if we're just going to dump
>> its ACL?

> I think I'm coming around to agree with that, but it seems like it'd be
> better to look at each component and say "we know X is safe, so we won't
> lock the table if we're only doing X" rather than saying "we only need to
> lock the table for case X".

Agreed. I did not realize you'd broken down the aspects of an object so
finely in pg_dump, but since you have, this is a good way to approach it.

> When considering the components:

> - DEFINITION
> - DATA
> [ obviously need lock ]

> - COMMENT

> Shouldn't require a lock, only uses a relatively simple query against
> pg_description.

> - SECLABEL

> Similar to COMMENT, shouldn't require a lock.

> - ACL

> ACL info is collected from pg_class relacl without any server-side
> functions being used which would impact the result.

> - POLICY

> Uses pg_get_expr(), which at least gets the relation name from
> SysCache, so we'll want to lock the table.

> - USERMAP

> Uses pg_options_to_table(), but I don't think that actually uses
> SysCache at all, it's just taking the array provided and builds a
> table out of it, so I think this case is ok.

USERMAP seems a bit squishy and easily broken, perhaps. Not sure there's
an advantage to distinguishing this case --- why did you break it out
from DEFINITION to start with? Also, AFAICS, it does not apply to tables
which are the only things we lock anyway.

Seems reasonable otherwise.

> Of course, the pg_dump would still end up including the ACLs for
> pg_authid and whatever other tables the user has changed the ACLs on and
> errors will be thrown during restore if the restore is done with a
> non-superuser.

Right, but at least you have the option of ignoring such errors.

regards, tom lane