On Tue, May 3, 2016 at 8:34 PM, Stephen Frost <sfr...@snowman.net> wrote:

> * Rushabh Lathia (rushabh.lat...@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
> 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).

If its limitation that if someone is using pg_dump as a non-superuser, it
should be limiting the tables using -n, then better we document that. But
I don't think this is valid limitation. Comments ?

> > 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 ==
> >
> > 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).

Right, condition should check for (DUMP_COMPONENT_DEFINITION ||

I am confused, in getTables() we executing LOCK TABLE, which holds the
share lock on that table till we get around to dumping them ( and that
dumping data or gathering the ACLs).  isn't that right ?

> Thanks!
> Stephen

Rushabh Lathia

Reply via email to