Обсуждение: dumping database privileges broken in 9.6

Поиск
Список
Период
Сортировка

dumping database privileges broken in 9.6

От
Peter Eisentraut
Дата:
Do this:

CREATE DATABASE test1;
REVOKE CONNECT ON DATABASE test1 FROM PUBLIC;

Run pg_dumpall.

In 9.5, this produces

CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
REVOKE ALL ON DATABASE test1 FROM PUBLIC;
REVOKE ALL ON DATABASE test1 FROM peter;
GRANT ALL ON DATABASE test1 TO peter;
GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;

In 9.6, this produces only

CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
GRANT ALL ON DATABASE test1 TO peter;

Note that the REVOKE statements are missing.  This does not correctly 
recreate the original state.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: dumping database privileges broken in 9.6

От
Robert Haas
Дата:
On Tue, Jun 28, 2016 at 11:12 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> Do this:
>
> CREATE DATABASE test1;
> REVOKE CONNECT ON DATABASE test1 FROM PUBLIC;
>
> Run pg_dumpall.
>
> In 9.5, this produces
>
> CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> REVOKE ALL ON DATABASE test1 FROM PUBLIC;
> REVOKE ALL ON DATABASE test1 FROM peter;
> GRANT ALL ON DATABASE test1 TO peter;
> GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
>
> In 9.6, this produces only
>
> CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> GRANT ALL ON DATABASE test1 TO peter;
>
> Note that the REVOKE statements are missing.  This does not correctly
> recreate the original state.

If I were a betting man, I'd bet that one of Stephen Frost's pg_dump
commits broke this.  But we'd have to bisect to be sure.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: dumping database privileges broken in 9.6

От
Stephen Frost
Дата:
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Tue, Jun 28, 2016 at 11:12 PM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
> > Do this:
> >
> > CREATE DATABASE test1;
> > REVOKE CONNECT ON DATABASE test1 FROM PUBLIC;
> >
> > Run pg_dumpall.
> >
> > In 9.5, this produces
> >
> > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > REVOKE ALL ON DATABASE test1 FROM PUBLIC;
> > REVOKE ALL ON DATABASE test1 FROM peter;
> > GRANT ALL ON DATABASE test1 TO peter;
> > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> >
> > In 9.6, this produces only
> >
> > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> > GRANT ALL ON DATABASE test1 TO peter;
> >
> > Note that the REVOKE statements are missing.  This does not correctly
> > recreate the original state.
>
> If I were a betting man, I'd bet that one of Stephen Frost's pg_dump
> commits broke this.  But we'd have to bisect to be sure.

Wouldn't be too surprising.  I'm planning to look into this a bit later
today.

Thanks!

Stephen

Re: dumping database privileges broken in 9.6

От
Stephen Frost
Дата:
Peter,

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> Do this:
>
> CREATE DATABASE test1;
> REVOKE CONNECT ON DATABASE test1 FROM PUBLIC;
>
> Run pg_dumpall.
>
> In 9.5, this produces
>
> CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> REVOKE ALL ON DATABASE test1 FROM PUBLIC;
> REVOKE ALL ON DATABASE test1 FROM peter;
> GRANT ALL ON DATABASE test1 TO peter;
> GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
>
> In 9.6, this produces only
>
> CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> GRANT ALL ON DATABASE test1 TO peter;
>
> Note that the REVOKE statements are missing.  This does not
> correctly recreate the original state.

I see what happened here, the query in dumpCreateDB() needs to be
adjusted to pull the default information to then pass to
buildACLComments(), similar to how the objects handled by pg_dump work.
The oversight was in thinking that databases didn't have any default
rights granted, which clearly isn't correct.

I'll take care of that in the next day or so and add an appropriate
regression test.

Thanks!

Stephen

Re: dumping database privileges broken in 9.6

От
Noah Misch
Дата:
On Wed, Jun 29, 2016 at 11:50:17AM -0400, Stephen Frost wrote:
> * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> > Do this:
> > 
> > CREATE DATABASE test1;
> > REVOKE CONNECT ON DATABASE test1 FROM PUBLIC;
> > 
> > Run pg_dumpall.
> > 
> > In 9.5, this produces
> > 
> > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > REVOKE ALL ON DATABASE test1 FROM PUBLIC;
> > REVOKE ALL ON DATABASE test1 FROM peter;
> > GRANT ALL ON DATABASE test1 TO peter;
> > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> > 
> > In 9.6, this produces only
> > 
> > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> > GRANT ALL ON DATABASE test1 TO peter;
> > 
> > Note that the REVOKE statements are missing.  This does not
> > correctly recreate the original state.
> 
> I see what happened here, the query in dumpCreateDB() needs to be
> adjusted to pull the default information to then pass to
> buildACLComments(), similar to how the objects handled by pg_dump work.
> The oversight was in thinking that databases didn't have any default
> rights granted, which clearly isn't correct.
> 
> I'll take care of that in the next day or so and add an appropriate
> regression test.

This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
a status update within 24 hours, and include a date for your subsequent status
update.  Refer to the policy on open item ownership:
http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com



Re: dumping database privileges broken in 9.6

От
Stephen Frost
Дата:
Noah, all,

On Saturday, July 2, 2016, Noah Misch <noah@leadboat.com> wrote:
On Wed, Jun 29, 2016 at 11:50:17AM -0400, Stephen Frost wrote:
> * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> > Do this:
> >
> > CREATE DATABASE test1;
> > REVOKE CONNECT ON DATABASE test1 FROM PUBLIC;
> >
> > Run pg_dumpall.
> >
> > In 9.5, this produces
> >
> > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > REVOKE ALL ON DATABASE test1 FROM PUBLIC;
> > REVOKE ALL ON DATABASE test1 FROM peter;
> > GRANT ALL ON DATABASE test1 TO peter;
> > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> >
> > In 9.6, this produces only
> >
> > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> > GRANT ALL ON DATABASE test1 TO peter;
> >
> > Note that the REVOKE statements are missing.  This does not
> > correctly recreate the original state.
>
> I see what happened here, the query in dumpCreateDB() needs to be
> adjusted to pull the default information to then pass to
> buildACLComments(), similar to how the objects handled by pg_dump work.
> The oversight was in thinking that databases didn't have any default
> rights granted, which clearly isn't correct.
>
> I'll take care of that in the next day or so and add an appropriate
> regression test.

This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
a status update within 24 hours, and include a date for your subsequent status
update.  Refer to the policy on open item ownership:
http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com

Will work on it tomorrow but for a deadline for next status update, I'll say Tuesday (which I expect is when I'll commit the fix) as it's a holiday weekend in the US. 

Thanks!

Stephen

Re: dumping database privileges broken in 9.6

От
Stephen Frost
Дата:
All,

* Noah Misch (noah@leadboat.com) wrote:
> On Wed, Jun 29, 2016 at 11:50:17AM -0400, Stephen Frost wrote:
> > * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> > > Do this:
> > >
> > > CREATE DATABASE test1;
> > > REVOKE CONNECT ON DATABASE test1 FROM PUBLIC;
> > >
> > > Run pg_dumpall.
> > >
> > > In 9.5, this produces
> > >
> > > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > > REVOKE ALL ON DATABASE test1 FROM PUBLIC;
> > > REVOKE ALL ON DATABASE test1 FROM peter;
> > > GRANT ALL ON DATABASE test1 TO peter;
> > > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> > >
> > > In 9.6, this produces only
> > >
> > > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> > > GRANT ALL ON DATABASE test1 TO peter;
> > >
> > > Note that the REVOKE statements are missing.  This does not
> > > correctly recreate the original state.
> >
> > I see what happened here, the query in dumpCreateDB() needs to be
> > adjusted to pull the default information to then pass to
> > buildACLComments(), similar to how the objects handled by pg_dump work.
> > The oversight was in thinking that databases didn't have any default
> > rights granted, which clearly isn't correct.
> >
> > I'll take care of that in the next day or so and add an appropriate
> > regression test.
>
> This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
> a status update within 24 hours, and include a date for your subsequent status
> update.  Refer to the policy on open item ownership:
> http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com

I've not forgotten about this and have an initial patch, but I'm
considering if I like the way template0/template1 are handled.
Specifically, we don't currently record their initdb-set privileges into
pg_init_privs (unlike all other objects with initial privileges).  This
is complicated by the idea that template1 could be dropped/recreated
(ending up with a different OID in the process).

More to come tomorrow.

Thanks!

Stephen

Re: dumping database privileges broken in 9.6

От
Noah Misch
Дата:
On Wed, Jul 06, 2016 at 07:03:33PM -0400, Stephen Frost wrote:
> * Noah Misch (noah@leadboat.com) wrote:
> > On Wed, Jun 29, 2016 at 11:50:17AM -0400, Stephen Frost wrote:
> > > * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> > > > Do this:
> > > > 
> > > > CREATE DATABASE test1;
> > > > REVOKE CONNECT ON DATABASE test1 FROM PUBLIC;
> > > > 
> > > > Run pg_dumpall.
> > > > 
> > > > In 9.5, this produces
> > > > 
> > > > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > > > REVOKE ALL ON DATABASE test1 FROM PUBLIC;
> > > > REVOKE ALL ON DATABASE test1 FROM peter;
> > > > GRANT ALL ON DATABASE test1 TO peter;
> > > > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> > > > 
> > > > In 9.6, this produces only
> > > > 
> > > > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > > > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> > > > GRANT ALL ON DATABASE test1 TO peter;
> > > > 
> > > > Note that the REVOKE statements are missing.  This does not
> > > > correctly recreate the original state.
> > > 
> > > I see what happened here, the query in dumpCreateDB() needs to be
> > > adjusted to pull the default information to then pass to
> > > buildACLComments(), similar to how the objects handled by pg_dump work.
> > > The oversight was in thinking that databases didn't have any default
> > > rights granted, which clearly isn't correct.
> > > 
> > > I'll take care of that in the next day or so and add an appropriate
> > > regression test.
> > 
> > This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
> > a status update within 24 hours, and include a date for your subsequent status
> > update.  Refer to the policy on open item ownership:
> > http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com
> 
> I've not forgotten about this and have an initial patch, but I'm
> considering if I like the way template0/template1 are handled.
> Specifically, we don't currently record their initdb-set privileges into
> pg_init_privs (unlike all other objects with initial privileges).  This
> is complicated by the idea that template1 could be dropped/recreated
> (ending up with a different OID in the process).
> 
> More to come tomorrow.

This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
a status update within 24 hours, and include a date for your subsequent status
update.  Refer to the policy on open item ownership:
http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com



Re: dumping database privileges broken in 9.6

От
Stephen Frost
Дата:
* Noah Misch (noah@leadboat.com) wrote:
> This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
> a status update within 24 hours, and include a date for your subsequent status
> update.  Refer to the policy on open item ownership:
> http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com

Unfortunately, not going to make any further progress on this tonight or
over the weekend as I'm going to be out of town.  I believe I've
convinced myself that adding a template1 entry to pg_init_privs will be
both sufficient and produce the correct results, along with adjusting
the query in pg_dumpall to join through it.  Will provide an update on
Monday.

Thanks!

Stephen

Re: dumping database privileges broken in 9.6

От
Stephen Frost
Дата:
All,

* Noah Misch (noah@leadboat.com) wrote:
> On Wed, Jun 29, 2016 at 11:50:17AM -0400, Stephen Frost wrote:
> > * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> > > Do this:
> > >
> > > CREATE DATABASE test1;
> > > REVOKE CONNECT ON DATABASE test1 FROM PUBLIC;
> > >
> > > Run pg_dumpall.
> > >
> > > In 9.5, this produces
> > >
> > > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > > REVOKE ALL ON DATABASE test1 FROM PUBLIC;
> > > REVOKE ALL ON DATABASE test1 FROM peter;
> > > GRANT ALL ON DATABASE test1 TO peter;
> > > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> > >
> > > In 9.6, this produces only
> > >
> > > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter;
> > > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC;
> > > GRANT ALL ON DATABASE test1 TO peter;
> > >
> > > Note that the REVOKE statements are missing.  This does not
> > > correctly recreate the original state.
> >
> > I see what happened here, the query in dumpCreateDB() needs to be
> > adjusted to pull the default information to then pass to
> > buildACLComments(), similar to how the objects handled by pg_dump work.
> > The oversight was in thinking that databases didn't have any default
> > rights granted, which clearly isn't correct.
> >
> > I'll take care of that in the next day or so and add an appropriate
> > regression test.
>
> This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
> a status update within 24 hours, and include a date for your subsequent status
> update.  Refer to the policy on open item ownership:
> http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com

Attached is a patch to address this.

After much consideration and deliberation, I went with the simpler
solution to simply dump out the database privileges based on what a new
creation of those privileges would yield, resulting in output similar to
pre-9.6.  We document that template1 is allowed to be dropped/recreated,
which greatly complicates using pg_init_privs to record and produce a
delta against the initdb-time values, as we lose the connection between
pg_init_privs and the "template1" database as soon as it is dropped
(something which can't be done with objects in that catalog).

Comments welcome.

Thanks!

Stephen

Вложения

Re: dumping database privileges broken in 9.6

От
Michael Paquier
Дата:
On Wed, Jul 13, 2016 at 5:18 AM, Stephen Frost <sfrost@snowman.net> wrote:
> Attached is a patch to address this.
>
> After much consideration and deliberation, I went with the simpler
> solution to simply dump out the database privileges based on what a new
> creation of those privileges would yield, resulting in output similar to
> pre-9.6.  We document that template1 is allowed to be dropped/recreated,
> which greatly complicates using pg_init_privs to record and produce a
> delta against the initdb-time values, as we lose the connection between
> pg_init_privs and the "template1" database as soon as it is dropped
> (something which can't be done with objects in that catalog).

+        "(SELECT pg_catalog.array_agg(acl) FROM (SELECT
pg_catalog.unnest(coalesce(datacl,pg_catalog.acldefault('d',datdba)))
AS acl "
+        "EXCEPT SELECT
pg_catalog.unnest(pg_catalog.acldefault('d',datdba))) as foo)"
+        "AS datacl,"
+        "(SELECT pg_catalog.array_agg(acl) FROM (SELECT
pg_catalog.unnest(pg_catalog.acldefault('d',datdba)) AS acl "
+        "EXCEPT SELECT
pg_catalog.unnest(coalesce(datacl,pg_catalog.acldefault('d',datdba))))
as foo)"
+        "AS rdatacl,"
It took me some time to understand that those are the GRANT and REVOKE
ACLs separated into two columns to get advantage of buildACLCommands..
-- 
Michael



Re: dumping database privileges broken in 9.6

От
Noah Misch
Дата:
On Sat, Jul 09, 2016 at 12:55:33AM -0400, Stephen Frost wrote:
> * Noah Misch (noah@leadboat.com) wrote:
> > This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
> > a status update within 24 hours, and include a date for your subsequent status
> > update.  Refer to the policy on open item ownership:
> > http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com
> 
> Unfortunately, not going to make any further progress on this tonight or
> over the weekend as I'm going to be out of town.  I believe I've
> convinced myself that adding a template1 entry to pg_init_privs will be
> both sufficient and produce the correct results, along with adjusting
> the query in pg_dumpall to join through it.  Will provide an update on
> Monday.

This PostgreSQL 9.6 open item is long past due for your status update.  Kindly
send a status update within 24 hours, and include a date for your subsequent
status update.  (Your Tuesday posting lacked a date.)



Re: dumping database privileges broken in 9.6

От
Stephen Frost
Дата:
* Noah Misch (noah@leadboat.com) wrote:
> On Sat, Jul 09, 2016 at 12:55:33AM -0400, Stephen Frost wrote:
> > * Noah Misch (noah@leadboat.com) wrote:
> > > This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
> > > a status update within 24 hours, and include a date for your subsequent status
> > > update.  Refer to the policy on open item ownership:
> > > http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com
> >
> > Unfortunately, not going to make any further progress on this tonight or
> > over the weekend as I'm going to be out of town.  I believe I've
> > convinced myself that adding a template1 entry to pg_init_privs will be
> > both sufficient and produce the correct results, along with adjusting
> > the query in pg_dumpall to join through it.  Will provide an update on
> > Monday.
>
> This PostgreSQL 9.6 open item is long past due for your status update.  Kindly
> send a status update within 24 hours, and include a date for your subsequent
> status update.  (Your Tuesday posting lacked a date.)

Yeah, I realized that tablespaces have the same issue and have updated
the patch to address them as well, in the same way.

Going through and doing testing now.  Unfortunately, it doesn't look
like adding in testing of tablespaces into the TAP tests would be very
easy (the only TAP test that deals with tablespaces that I found was
pg_basebackup and that looks rather grotty..), so I'm not planning to do
that, at least not at this time.

As such, I'm planning to commit the patch with the fix+regression test
for database ACLs and the fix for tablespace ACLs either later today
or tomorrow.

Thanks!

Stephen

Re: dumping database privileges broken in 9.6

От
Noah Misch
Дата:
On Fri, Jul 15, 2016 at 03:46:17PM -0400, Stephen Frost wrote:
> * Noah Misch (noah@leadboat.com) wrote:
> > On Sat, Jul 09, 2016 at 12:55:33AM -0400, Stephen Frost wrote:
> > > * Noah Misch (noah@leadboat.com) wrote:
> > > > This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
> > > > a status update within 24 hours, and include a date for your subsequent status
> > > > update.  Refer to the policy on open item ownership:
> > > > http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com
> > > 
> > > Unfortunately, not going to make any further progress on this tonight or
> > > over the weekend as I'm going to be out of town.  I believe I've
> > > convinced myself that adding a template1 entry to pg_init_privs will be
> > > both sufficient and produce the correct results, along with adjusting
> > > the query in pg_dumpall to join through it.  Will provide an update on
> > > Monday.
> > 
> > This PostgreSQL 9.6 open item is long past due for your status update.  Kindly
> > send a status update within 24 hours, and include a date for your subsequent
> > status update.  (Your Tuesday posting lacked a date.)
> 
> Yeah, I realized that tablespaces have the same issue and have updated
> the patch to address them as well, in the same way.
> 
> Going through and doing testing now.  Unfortunately, it doesn't look
> like adding in testing of tablespaces into the TAP tests would be very
> easy (the only TAP test that deals with tablespaces that I found was
> pg_basebackup and that looks rather grotty..), so I'm not planning to do
> that, at least not at this time.
> 
> As such, I'm planning to commit the patch with the fix+regression test
> for database ACLs and the fix for tablespace ACLs either later today
> or tomorrow.

Does commit 47f5bb9 fully fix this open item?



Re: dumping database privileges broken in 9.6

От
Michael Paquier
Дата:
On Sat, Jul 16, 2016 at 4:46 AM, Stephen Frost <sfrost@snowman.net> wrote:
> Going through and doing testing now.  Unfortunately, it doesn't look
> like adding in testing of tablespaces into the TAP tests would be very
> easy (the only TAP test that deals with tablespaces that I found was
> pg_basebackup and that looks rather grotty..), so I'm not planning to do
> that, at least not at this time.

The cleanest way to handle that in PostgresNode would be to have a
dedicated routine calling psql -c 'create tablespace' with tablespaces
located in a folder $basedir/tbspace. And on top of that there should
be the tablespace metadata saved in the context of the test, with a
pair of (tbspc name, location). But I think that we'd need first
stronger arguments (take more use cases) to introduce such an
extension of the test APIs.
-- 
Michael



Re: dumping database privileges broken in 9.6

От
Stephen Frost
Дата:
Noah,

On Monday, July 18, 2016, Noah Misch <noah@leadboat.com> wrote:
On Fri, Jul 15, 2016 at 03:46:17PM -0400, Stephen Frost wrote:
> * Noah Misch (noah@leadboat.com) wrote:
> > On Sat, Jul 09, 2016 at 12:55:33AM -0400, Stephen Frost wrote:
> > > * Noah Misch (noah@leadboat.com) wrote:
> > > > This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
> > > > a status update within 24 hours, and include a date for your subsequent status
> > > > update.  Refer to the policy on open item ownership:
> > > > http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com
> > >
> > > Unfortunately, not going to make any further progress on this tonight or
> > > over the weekend as I'm going to be out of town.  I believe I've
> > > convinced myself that adding a template1 entry to pg_init_privs will be
> > > both sufficient and produce the correct results, along with adjusting
> > > the query in pg_dumpall to join through it.  Will provide an update on
> > > Monday.
> >
> > This PostgreSQL 9.6 open item is long past due for your status update.  Kindly
> > send a status update within 24 hours, and include a date for your subsequent
> > status update.  (Your Tuesday posting lacked a date.)
>
> Yeah, I realized that tablespaces have the same issue and have updated
> the patch to address them as well, in the same way.
>
> Going through and doing testing now.  Unfortunately, it doesn't look
> like adding in testing of tablespaces into the TAP tests would be very
> easy (the only TAP test that deals with tablespaces that I found was
> pg_basebackup and that looks rather grotty..), so I'm not planning to do
> that, at least not at this time.
>
> As such, I'm planning to commit the patch with the fix+regression test
> for database ACLs and the fix for tablespace ACLs either later today
> or tomorrow.

Does commit 47f5bb9 fully fix this open item?

Yes, it does. Apologies for not closing it. 

Thanks!

Stephen 

Re: dumping database privileges broken in 9.6

От
Stephen Frost
Дата:
* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Sat, Jul 16, 2016 at 4:46 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > Going through and doing testing now.  Unfortunately, it doesn't look
> > like adding in testing of tablespaces into the TAP tests would be very
> > easy (the only TAP test that deals with tablespaces that I found was
> > pg_basebackup and that looks rather grotty..), so I'm not planning to do
> > that, at least not at this time.
>
> The cleanest way to handle that in PostgresNode would be to have a
> dedicated routine calling psql -c 'create tablespace' with tablespaces
> located in a folder $basedir/tbspace. And on top of that there should
> be the tablespace metadata saved in the context of the test, with a
> pair of (tbspc name, location). But I think that we'd need first
> stronger arguments (take more use cases) to introduce such an
> extension of the test APIs.

The way pg_basebackup handles this is to use TestLib::tempdir_short and
create a symlink with it, then to call psql to create the tablespace.  I
don't have any problem using $basedir/tbspace instead though.

What I was thinking is that we'd add a 'create_tablespace' or similar
routine to PostgresNode and then make the pg_basebackup and pg_dump
tests use it.  Otherwise, I suspect the next person who ends up writing
a 'create tablespace' into the TAP tests will use yet another location
and/or method.

Thanks!

Stephen

Re: dumping database privileges broken in 9.6

От
Stephen Frost
Дата:
* Noah Misch (noah@leadboat.com) wrote:
> On Fri, Jul 15, 2016 at 03:46:17PM -0400, Stephen Frost wrote:
> > * Noah Misch (noah@leadboat.com) wrote:
> > > On Sat, Jul 09, 2016 at 12:55:33AM -0400, Stephen Frost wrote:
> > > > * Noah Misch (noah@leadboat.com) wrote:
> > > > > This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
> > > > > a status update within 24 hours, and include a date for your subsequent status
> > > > > update.  Refer to the policy on open item ownership:
> > > > > http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com
> > > >
> > > > Unfortunately, not going to make any further progress on this tonight or
> > > > over the weekend as I'm going to be out of town.  I believe I've
> > > > convinced myself that adding a template1 entry to pg_init_privs will be
> > > > both sufficient and produce the correct results, along with adjusting
> > > > the query in pg_dumpall to join through it.  Will provide an update on
> > > > Monday.
> > >
> > > This PostgreSQL 9.6 open item is long past due for your status update.  Kindly
> > > send a status update within 24 hours, and include a date for your subsequent
> > > status update.  (Your Tuesday posting lacked a date.)
> >
> > Yeah, I realized that tablespaces have the same issue and have updated
> > the patch to address them as well, in the same way.
> >
> > Going through and doing testing now.  Unfortunately, it doesn't look
> > like adding in testing of tablespaces into the TAP tests would be very
> > easy (the only TAP test that deals with tablespaces that I found was
> > pg_basebackup and that looks rather grotty..), so I'm not planning to do
> > that, at least not at this time.
> >
> > As such, I'm planning to commit the patch with the fix+regression test
> > for database ACLs and the fix for tablespace ACLs either later today
> > or tomorrow.
>
> Does commit 47f5bb9 fully fix this open item?

Yes, I've updated the open items wiki.

Thanks!

Stephen