* Rushabh Lathia (rushabh.lathia@gmail.com) wrote:
> With commit a9f0e8e5a2e779a888988cb64479a6723f668c84, now pg_dump, use a
> bitmap
> to represent what to include. With this commit if non-super user is unable
> to perform the dump.
[...]
> pg_dump: [archiver (db)] query was: LOCK TABLE pg_catalog.pg_authid IN
> ACCESS SHARE MODE
This does get addressed, in a way, in one of the patches that I've
currently got pending for pg_dump. That particular change is actually
one for performance and therefore it's only going to help in cases where
none of the catalog tables have had their ACLs changed.
If the ACL has changed for any catalog table then pg_dump will still try
to LOCK the table, because we're going to dump out information about
that table (the ACLs on it). I'm not sure if that's really an issue or
not.. Generally, if you're using pg_dump as a non-superuser, I'd expect
you to be limiting the tables you're dumping to ones you have access to
normally (using -n).
> getTables() take read-lock target tables to make sure they aren't DROPPED
> or altered in schema before we get around to dumping them. Here it having
> below condition to take a lock:
>
> if (tblinfo[i].dobj.dump && tblinfo[i].relkind == RELKIND_RELATION)
>
> which need to replace with:
>
> if ((tblinfo[i].dobj.dump & DUMP_COMPONENT_DEFINITION) &&
> tblinfo[i].relkind == RELKIND_RELATION)
>
> PFA patch to fix the issue.
I don't think we want to limit the cases where we take a lock to just
when we're dumping out the table definition.. Consider what happens if
someone drops the table before we get to dumping out the data in the
table, or gathering the ACLs on it (which happens much, much later).
Thanks!
Stephen
