On Mon, Apr 25, 2016 at 12:39:09AM -0400, Stephen Frost wrote: > * Noah Misch (n...@leadboat.com) wrote: > > On Fri, Apr 22, 2016 at 12:31:41PM -0400, Stephen Frost wrote: > > > After looking through the code a bit, I realized that there are a lot of > > > object types which don't have ACLs at all but which exist in pg_catalog > > > and were being analyzed because the bitmask for pg_catalog included ACLs > > > and therefore was non-zero. > > > > > > Clearing that bit for object types which don't have ACLs improved the > > > performance for empty databases quite a bit (from about 3s to a bit > > > under 1s on my laptop). That's a 42-line patch, with comment lines > > > being half of that, which I'll push once I've looked into the other > > > concerns which were brought up on this thread. > > > > That's good news. > > Attached patch-set includes this change in patch #2.
Timings for the 100-database pg_dumpall: HEAD: 131s HEAD+patch: 33s 9.5: 8.6s Nice improvement for such a simple patch. > Patch #1 is the fix for the incorrect WHERE clause. I confirmed that this fixed dumping of the function ACL in my test case. > For languages, I believe that means that we simply need to modify the > selectDumpableProcLang() function to use the same default we use for the > 'pg_catalog' namespace- DUMP_COMPONENT_ACL, instead of > DUMP_COMPONENT_NONE. Makes sense. > What's not clear to me is what, if any, issue there is with namespaces. As far as I know, none. The current behavior doesn't match the commit message, but I think the current behavior is better. > Certainly, in my testing at least, if you do: > > REVOKE CREATE ON SCHEMA public FROM public; > > Then you get the appropriate commands from pg_dump to implement the > resulting ACLs on the public schema. If you change the permissions back > to match what is there at initdb-time (or you just don't change them), > then there aren't any GRANT or REVOKE commands from pg_dump, but that's > correct, isn't it? Good question. I think it's fine and possibly even optimal. One can imagine other designs that remember whether any GRANT or REVOKE has happened since initdb, but I see no definite reason to prefer that alternative. > In the attached patch-set, patch #3 includes support for > > src/bin/pg_dump: make check > > implemented using the TAP testing system. There are a total of 360 > tests, generally covering: I like the structure of this test suite. > +# Start with 1 because of non-existant database test below > +# Test connecting to a non-existant database Spelling. -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers