Re: [HACKERS] pg_dump dump catalog ACLs

Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > The subquery comparing the OID of pg_class using only a condition on > relname seems wrong; wouldn't it fail or produce wrong results if > somebody creates a table named pg_class in another schema? I think you > should write the

Re: [HACKERS] pg_dump dump catalog ACLs

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost wrote: > > > diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c > > index 1267afb..4a9b1bf 100644 > > --- a/src/bin/pg_dump/pg_dump.c > > +++ b/src/bin/pg_dump/pg_dump.c > > @@ -14992,9 +14992,10 @@

Re: [HACKERS] pg_dump dump catalog ACLs

Stephen Frost wrote: > diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c > index 1267afb..4a9b1bf 100644 > --- a/src/bin/pg_dump/pg_dump.c > +++ b/src/bin/pg_dump/pg_dump.c > @@ -14992,9 +14992,10 @@ dumpTable(Archive *fout, TableInfo *tbinfo) >

Re: [HACKERS] pg_dump dump catalog ACLs

* Noah Misch (n...@leadboat.com) wrote: > On Tue, Apr 05, 2016 at 05:50:18PM -0400, Stephen Frost wrote: > > @@ -14187,18 +14869,65 @@ dumpTable(Archive *fout, TableInfo *tbinfo) > > > + "FROM > > pg_catalog.pg_attribute at " > > +

Re: [HACKERS] pg_dump dump catalog ACLs

* Stephen Frost (sfr...@snowman.net) wrote: > In any case, as I was saying, that's far closer to 9.5 run-time. I've > not measured the time added when things like TRANSFORMs were added, but > it wouldn't surprise me if adding a new query for every database to > pg_dump adds something similar to

Re: [HACKERS] pg_dump dump catalog ACLs

* Noah Misch (n...@leadboat.com) wrote: > On Wed, May 04, 2016 at 08:14:55AM -0400, Stephen Frost wrote: > > * Noah Misch (n...@leadboat.com) wrote: > > > On Mon, Apr 25, 2016 at 12:39:09AM -0400, Stephen Frost wrote: > > > > * Noah Misch (n...@leadboat.com) wrote: > > > > > On Fri, Apr 22, 2016

Re: [HACKERS] pg_dump dump catalog ACLs

On Wed, May 04, 2016 at 08:14:55AM -0400, Stephen Frost wrote: > * Noah Misch (n...@leadboat.com) wrote: > > 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: > > > > >

Re: [HACKERS] pg_dump dump catalog ACLs

Noah, all, * Stephen Frost (sfr...@snowman.net) wrote: > The test suite is now covering 57% of pg_dump.c. I was hoping to get > that number higher, but time marches on and more tests can certainly be > added later. I've managed to get the test suite up another 10%, to 67% of pg_dump.c. Still

Re: [HACKERS] pg_dump dump catalog ACLs

Noah, * Noah Misch (n...@leadboat.com) wrote: > 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

Re: [HACKERS] pg_dump dump catalog ACLs

On Fri, Apr 22, 2016 at 3:30 AM, Peter Geoghegan wrote: > On Fri, Apr 22, 2016 at 12:25 AM, Noah Misch wrote: >> Folks run clusters with ~1000 databases; we previously accepted at least one >> complex performance improvement[1] based on that use case. On the

Re: [HACKERS] pg_dump dump catalog ACLs

Alvaro, On Wednesday, April 27, 2016, Alvaro Herrera wrote: > Stephen Frost wrote: > > > The current challenge I've been trying to find a solution to is testing > the > > extension handling. Only core extensions (plpgsql, plperl, etc) are > > available when the TAP

Re: [HACKERS] pg_dump dump catalog ACLs

Stephen Frost wrote: > The current challenge I've been trying to find a solution to is testing the > extension handling. Only core extensions (plpgsql, plperl, etc) are > available when the TAP tests are running for pg_dump, it seems. I certainly > don't want to add a testing extension in such a

Re: [HACKERS] pg_dump dump catalog ACLs

Noah, On Wednesday, April 27, 2016, Noah Misch wrote: > A later thought: > > On Mon, Apr 25, 2016 at 12:39:09AM -0400, Stephen Frost wrote: > > src/bin/pg_dump: make check > > > > implemented using the TAP testing system. There are a total of 360 > > tests, generally

Re: [HACKERS] pg_dump dump catalog ACLs

A later thought: On Mon, Apr 25, 2016 at 12:39:09AM -0400, Stephen Frost wrote: > src/bin/pg_dump: make check > > implemented using the TAP testing system. There are a total of 360 > tests, generally covering: > > Various invalid sets of command-line options. > > Valid command-line options

Re: [HACKERS] pg_dump dump catalog ACLs

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

Re: [HACKERS] pg_dump dump catalog ACLs

Noah, all, * 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

Re: [HACKERS] pg_dump dump catalog ACLs

On Fri, Apr 22, 2016 at 08:24:53PM -0400, Stephen Frost wrote: > * Noah Misch (n...@leadboat.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

Re: [HACKERS] pg_dump dump catalog ACLs

On Fri, Apr 22, 2016 at 12:31:41PM -0400, Stephen Frost wrote: > * Noah Misch (n...@leadboat.com) wrote: > > On Wed, Apr 20, 2016 at 10:50:21PM -0400, Stephen Frost wrote: > > > I'm certainly open to improving these issues now if we agree that they > > > should be fixed for 9.6. If we don't want

Re: [HACKERS] pg_dump dump catalog ACLs

Noah, all, * Noah Misch (n...@leadboat.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

Re: [HACKERS] pg_dump dump catalog ACLs

* Noah Misch (n...@leadboat.com) wrote: > On Wed, Apr 20, 2016 at 10:50:21PM -0400, Stephen Frost wrote: > > I'm certainly open to improving these issues now if we agree that they > > should be fixed for 9.6. If we don't want to include such changes in 9.6 > > then I will propose then for

Re: [HACKERS] pg_dump dump catalog ACLs

On Fri, Apr 22, 2016 at 12:25 AM, Noah Misch wrote: > Folks run clusters with ~1000 databases; we previously accepted at least one > complex performance improvement[1] based on that use case. On the faster of > the two machines I tested, the present thread's commits slowed

Re: [HACKERS] pg_dump dump catalog ACLs

On Wed, Apr 20, 2016 at 10:50:21PM -0400, Stephen Frost wrote: > On Wednesday, April 20, 2016, Noah Misch wrote: > > On Wed, Apr 20, 2016 at 11:12:44AM -0400, Stephen Frost wrote: > > > > On Sun, Apr 17, 2016 at 11:02:28PM -0400, Noah Misch wrote: > > > > > (3) pg_dumpall

Re: [HACKERS] pg_dump dump catalog ACLs

Noah, On Wednesday, April 20, 2016, Noah Misch wrote: > On Wed, Apr 20, 2016 at 11:12:44AM -0400, Stephen Frost wrote: > > * Noah Misch (n...@leadboat.com ) wrote: > > > On Sun, Apr 17, 2016 at 11:02:28PM -0400, Noah Misch wrote: > > > > (3) pg_dumpall became

Re: [HACKERS] pg_dump dump catalog ACLs

On Wed, Apr 20, 2016 at 11:12:44AM -0400, Stephen Frost wrote: > * Noah Misch (n...@leadboat.com) wrote: > > On Sun, Apr 17, 2016 at 11:02:28PM -0400, Noah Misch wrote: > > > (3) pg_dumpall became much slower around the time of these commits. On > > > one > > > machine (POWER7 3.55 GHz), a

Re: [HACKERS] pg_dump dump catalog ACLs

Noah, all, * Noah Misch (n...@leadboat.com) wrote: > On Sun, Apr 17, 2016 at 11:02:28PM -0400, Noah Misch wrote: > > On Tue, Apr 05, 2016 at 05:50:18PM -0400, Stephen Frost wrote: > > > I'll be doing more testing, review and clean-up (there are some > > > whitespace only changes in the later

Re: [HACKERS] pg_dump dump catalog ACLs

On Sun, Apr 17, 2016 at 11:02:28PM -0400, Noah Misch wrote: > On Tue, Apr 05, 2016 at 05:50:18PM -0400, Stephen Frost wrote: > > I'll be doing more testing, review and clean-up (there are some > > whitespace only changes in the later patches that really should be > > included in the earlier

Re: [HACKERS] pg_dump dump catalog ACLs

On Tue, Apr 05, 2016 at 05:50:18PM -0400, Stephen Frost wrote: > I'll be doing more testing, review and clean-up (there are some > whitespace only changes in the later patches that really should be > included in the earlier patches where the code was actually changed) > tonight with plans to push

Re: [HACKERS] pg_dump dump catalog ACLs

On Wed, Mar 30, 2016 at 1:35 PM, Jose Luis Tallon wrote: > DESIGN/DOCUMENTATION > * int4 for the "objsubid" field? and int2 would surely suffice, given that we > only allow up to 1600 columns ... if it's a matter of alignment, it would be > interesting to say so

Re: [HACKERS] pg_dump dump catalog ACLs

Hi, Stephen! I'm signed to review this patch. At first, patch wasn't applied cleanly, it had a conflict at the end of system_views.sql. But that way very easy to fix. On Mon, Mar 14, 2016 at 7:00 PM, Stephen Frost wrote: > I've not included it in this patch, but my thinking

Re: [HACKERS] pg_dump dump catalog ACLs

Tom, all, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Joe Conway writes: > > Would it be a terrible idea to add some attribute to ACLs which can be > > used to indicate they should not be dumped (and supporting syntax)? > > Yes, we'd need some way to mark non-null ACLs as being

Re: [HACKERS] pg_dump dump catalog ACLs

* Joe Conway (m...@joeconway.com) wrote: > On 03/02/2016 12:54 PM, Stephen Frost wrote: > > * Joe Conway (m...@joeconway.com) wrote: > >> On 03/01/2016 08:00 AM, Tom Lane wrote: > >>> Yes, we'd need some way to mark non-null ACLs as being "built-in > >>> defaults". I do not see the need to have

Re: [HACKERS] pg_dump dump catalog ACLs

On Wed, Mar 2, 2016 at 2:44 PM, Joe Conway wrote: > On 03/02/2016 12:54 PM, Stephen Frost wrote: > > * Joe Conway (m...@joeconway.com) wrote: > >> On 03/01/2016 08:00 AM, Tom Lane wrote: > >>> Yes, we'd need some way to mark non-null ACLs as being "built-in > >>> defaults".

Re: [HACKERS] pg_dump dump catalog ACLs

On 03/02/2016 12:54 PM, Stephen Frost wrote: > * Joe Conway (m...@joeconway.com) wrote: >> On 03/01/2016 08:00 AM, Tom Lane wrote: >>> Yes, we'd need some way to mark non-null ACLs as being "built-in >>> defaults". I do not see the need to have SQL syntax supporting that >>> though. >> >> I was

Re: [HACKERS] pg_dump dump catalog ACLs

* David G. Johnston (david.g.johns...@gmail.com) wrote: > On Wed, Mar 2, 2016 at 1:54 PM, Stephen Frost wrote: > > Rather than have two aclitem[] columns in every catalog, since this > > information is only used by pg_dump and not during normal operation, we > > could use the

Re: [HACKERS] pg_dump dump catalog ACLs

On Wed, Mar 2, 2016 at 1:54 PM, Stephen Frost wrote: > * Joe Conway (m...@joeconway.com) wrote: > > On 03/01/2016 08:00 AM, Tom Lane wrote: > > > Joe Conway writes: > > >> Would it be a terrible idea to add some attribute to ACLs which can be > > >> used

Re: [HACKERS] pg_dump dump catalog ACLs

* Joe Conway (m...@joeconway.com) wrote: > On 03/01/2016 08:00 AM, Tom Lane wrote: > > Joe Conway writes: > >> Would it be a terrible idea to add some attribute to ACLs which can be > >> used to indicate they should not be dumped (and supporting syntax)? > > > > Yes, we'd

Re: [HACKERS] pg_dump dump catalog ACLs

On 03/01/2016 08:00 AM, Tom Lane wrote: > Joe Conway writes: >> Would it be a terrible idea to add some attribute to ACLs which can be >> used to indicate they should not be dumped (and supporting syntax)? > > Yes, we'd need some way to mark non-null ACLs as being "built-in >

Re: [HACKERS] pg_dump dump catalog ACLs

Joe Conway writes: > Would it be a terrible idea to add some attribute to ACLs which can be > used to indicate they should not be dumped (and supporting syntax)? Yes, we'd need some way to mark non-null ACLs as being "built-in defaults". I do not see the need to have SQL

Re: [HACKERS] pg_dump dump catalog ACLs

On 02/29/2016 08:52 PM, Tom Lane wrote: > Stephen Frost writes: >> As it turns out, there isn't such an issue as the default for functions >> is to allow PUBLIC to EXECUTE and therefore we don't dump out ACLs for >> most functions. The follow-on change to this patch is to

Re: [HACKERS] pg_dump dump catalog ACLs

Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> No, the point of it would be to not have pg_dump scripts overriding >> installed-by-default ACLs. A newer PG version might have different >> ideas about what those should be. I don't think this is exactly an

Re: [HACKERS] pg_dump dump catalog ACLs

* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> To make this work, you'd need a way to distinguish privileges installed > >> by initdb from those changed later. > > > To replicate whatever the current ACL is, we

Re: [HACKERS] pg_dump dump catalog ACLs

Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> To make this work, you'd need a way to distinguish privileges installed >> by initdb from those changed later. > To replicate whatever the current ACL is, we don't actually need to > make such a

Re: [HACKERS] pg_dump dump catalog ACLs

* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > Per discussion about the best approach to reduce the amount of > > superuser-only capabilities, this patch modifies pg_dump to dump out > > all ACLs which exist on objects in the pg_catalog schema. > > Um ...

Re: [HACKERS] pg_dump dump catalog ACLs

Stephen Frost writes: > Per discussion about the best approach to reduce the amount of > superuser-only capabilities, this patch modifies pg_dump to dump out > all ACLs which exist on objects in the pg_catalog schema. Um ... surely there are some of those that are installed

[HACKERS] pg_dump dump catalog ACLs

All, Per discussion about the best approach to reduce the amount of superuser-only capabilities, this patch modifies pg_dump to dump out all ACLs which exist on objects in the pg_catalog schema. With this change, follow-on trivial patches will remove explicit superuser() checks from functions