Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.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