Re: pg_dump dump catalog ACLs

Noah, all,

* Noah Misch (noah(at)leadboat(dot)com) wrote:
> On Tue, Apr 05, 2016 at 05:50:18PM -0400, Stephen Frost wrote:
> > Subject: [PATCH 4/5] In pg_dump, include pg_catalog and extension ACLs, if
> > changed
> >
> > Now that all of the infrastructure exists, add in the ability to
> > dump out the ACLs of the objects inside of pg_catalog or the ACLs
> > for objects which are members of extensions, but only if they have
> > been changed from their original values.
>
> I wrote the attached test script to verify which types of ACLs dump/reload
> covers. Based on the commit message, I expected these results:
>
> Dumped: type, class, attribute, proc, largeobject_metadata,
> foreign_data_wrapper, foreign_server,
> language(in-extension), namespace(in-extension)
> Not dumped: database, tablespace,
> language(from-initdb), namespace(from-initdb)
>
> Did I interpret the commit message correctly? The script gave these results:
>
> Dumped: type, class, attribute, namespace(from-initdb)
> Not dumped: database, tablespace, proc, language(from-initdb)

You interpreted the commit message correctly and in a number of cases
the correct results are generated, but there's an issue in the WHERE
clause being used for some of the object types. The earlier versions of
this patch adjusted the WHERE clause by using a sub-select, but later I
turned it into a LEFT JOIN and didn't correctly update the WHERE clause
to reflect that, so some kinds of ACL changes weren't being picked up.

That's relatively straight-forward to fix, but I'm going to put
together a series of TAP tests to go with these fixes. While I tested
various options and bits and pieces of the code while developing, this
really needs a proper test suite that runs through all of these
permutations with each change.

I'm planning to have that done by the end of the weekend.

Thanks!

Stephen