dumping database privileges broken in 9.6

Started by Peter Eisentrautover 9 years ago18 messages
#1Peter Eisentraut
peter.eisentraut@2ndquadrant.com

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#1)
Re: dumping database privileges broken in 9.6

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Stephen Frost
sfrost@snowman.net
In reply to: Robert Haas (#2)
Re: dumping database privileges broken in 9.6

* 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

#4Stephen Frost
sfrost@snowman.net
In reply to: Peter Eisentraut (#1)
Re: dumping database privileges broken in 9.6

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

#5Noah Misch
noah@leadboat.com
In reply to: Stephen Frost (#4)
Re: dumping database privileges broken in 9.6

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:
/messages/by-id/20160527025039.GA447393@tornado.leadboat.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Stephen Frost
sfrost@snowman.net
In reply to: Noah Misch (#5)
Re: dumping database privileges broken in 9.6

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 <javascript:;>)

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:

/messages/by-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

#7Stephen Frost
sfrost@snowman.net
In reply to: Noah Misch (#5)
Re: dumping database privileges broken in 9.6

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:
/messages/by-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

#8Noah Misch
noah@leadboat.com
In reply to: Stephen Frost (#7)
Re: dumping database privileges broken in 9.6

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:
/messages/by-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:
/messages/by-id/20160527025039.GA447393@tornado.leadboat.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Stephen Frost
sfrost@snowman.net
In reply to: Noah Misch (#8)
Re: dumping database privileges broken in 9.6

* 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:
/messages/by-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

#10Stephen Frost
sfrost@snowman.net
In reply to: Noah Misch (#5)
1 attachment(s)
Re: dumping database privileges broken in 9.6

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:
/messages/by-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

Attachments:

pg_dumpall_privs_v1.patchtext/x-diff; charset=us-asciiDownload
From fc87c0a07f37d7b4bbcf067d22d31467a511e865 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Tue, 12 Jul 2016 15:37:35 -0400
Subject: [PATCH] Correctly dump database ACLs

---
 src/bin/pg_dump/pg_dumpall.c     |  54 +++++++++++---
 src/bin/pg_dump/t/002_pg_dump.pl | 153 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 195 insertions(+), 12 deletions(-)

diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index d4fb03e..99c0f90 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1284,14 +1284,43 @@ dumpCreateDB(PGconn *conn)
 
 	PQclear(res);
 
-	/* Now collect all the information about databases to dump */
-	if (server_version >= 90300)
+
+	/*
+	 * Now collect all the information about databases to dump.
+	 *
+	 * For the database ACLs, as of 9.6, we extract both the positive (as
+	 * datacl) and negative (as rdatacl) ACLs, relative to the default ACL for
+	 * databases, which are then passed to buildACLCommands() below.
+	 *
+	 * See buildACLQueries() and buildACLCommands().
+	 *
+	 * Note that we do not support initial privileges (pg_init_privs) on
+	 * databases.
+	 */
+	if (server_version >= 90600)
+		res = executeQuery(conn,
+						   "SELECT datname, "
+						   "coalesce(rolname, (select rolname from pg_authid where oid=(select datdba from pg_database where datname='template0'))), "
+						   "pg_encoding_to_char(d.encoding), "
+						   "datcollate, datctype, datfrozenxid, datminmxid, "
+						   "datistemplate, "
+						   "(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,"
+						   "datconnlimit, "
+						   "(SELECT spcname FROM pg_tablespace t WHERE t.oid = d.dattablespace) AS dattablespace "
+			  "FROM pg_database d LEFT JOIN pg_authid u ON (datdba = u.oid) "
+						   "WHERE datallowconn ORDER BY 1");
+	else if (server_version >= 90300)
 		res = executeQuery(conn,
 						   "SELECT datname, "
 						   "coalesce(rolname, (select rolname from pg_authid where oid=(select datdba from pg_database where datname='template0'))), "
 						   "pg_encoding_to_char(d.encoding), "
 						   "datcollate, datctype, datfrozenxid, datminmxid, "
-						   "datistemplate, datacl, datconnlimit, "
+						   "datistemplate, datacl, '' as rdatacl, datconnlimit, "
 						   "(SELECT spcname FROM pg_tablespace t WHERE t.oid = d.dattablespace) AS dattablespace "
 			  "FROM pg_database d LEFT JOIN pg_authid u ON (datdba = u.oid) "
 						   "WHERE datallowconn ORDER BY 1");
@@ -1301,7 +1330,7 @@ dumpCreateDB(PGconn *conn)
 						   "coalesce(rolname, (select rolname from pg_authid where oid=(select datdba from pg_database where datname='template0'))), "
 						   "pg_encoding_to_char(d.encoding), "
 					  "datcollate, datctype, datfrozenxid, 0 AS datminmxid, "
-						   "datistemplate, datacl, datconnlimit, "
+						   "datistemplate, datacl, '' as rdatacl, datconnlimit, "
 						   "(SELECT spcname FROM pg_tablespace t WHERE t.oid = d.dattablespace) AS dattablespace "
 			  "FROM pg_database d LEFT JOIN pg_authid u ON (datdba = u.oid) "
 						   "WHERE datallowconn ORDER BY 1");
@@ -1311,7 +1340,7 @@ dumpCreateDB(PGconn *conn)
 						   "coalesce(rolname, (select rolname from pg_authid where oid=(select datdba from pg_database where datname='template0'))), "
 						   "pg_encoding_to_char(d.encoding), "
 						   "null::text AS datcollate, null::text AS datctype, datfrozenxid, 0 AS datminmxid, "
-						   "datistemplate, datacl, datconnlimit, "
+						   "datistemplate, datacl, '' as rdatacl, datconnlimit, "
 						   "(SELECT spcname FROM pg_tablespace t WHERE t.oid = d.dattablespace) AS dattablespace "
 			  "FROM pg_database d LEFT JOIN pg_authid u ON (datdba = u.oid) "
 						   "WHERE datallowconn ORDER BY 1");
@@ -1321,7 +1350,7 @@ dumpCreateDB(PGconn *conn)
 						   "coalesce(usename, (select usename from pg_shadow where usesysid=(select datdba from pg_database where datname='template0'))), "
 						   "pg_encoding_to_char(d.encoding), "
 						   "null::text AS datcollate, null::text AS datctype, datfrozenxid, 0 AS datminmxid, "
-						   "datistemplate, datacl, -1 as datconnlimit, "
+						   "datistemplate, datacl, '' as rdatacl, -1 as datconnlimit, "
 						   "(SELECT spcname FROM pg_tablespace t WHERE t.oid = d.dattablespace) AS dattablespace "
 		   "FROM pg_database d LEFT JOIN pg_shadow u ON (datdba = usesysid) "
 						   "WHERE datallowconn ORDER BY 1");
@@ -1331,7 +1360,7 @@ dumpCreateDB(PGconn *conn)
 						   "coalesce(usename, (select usename from pg_shadow where usesysid=(select datdba from pg_database where datname='template0'))), "
 						   "pg_encoding_to_char(d.encoding), "
 						   "null::text AS datcollate, null::text AS datctype, datfrozenxid, 0 AS datminmxid, "
-						   "datistemplate, datacl, -1 as datconnlimit, "
+						   "datistemplate, datacl, '' as rdatacl, -1 as datconnlimit, "
 						   "'pg_default' AS dattablespace "
 		   "FROM pg_database d LEFT JOIN pg_shadow u ON (datdba = usesysid) "
 						   "WHERE datallowconn ORDER BY 1");
@@ -1343,7 +1372,7 @@ dumpCreateDB(PGconn *conn)
 						   "(select usename from pg_shadow where usesysid=(select datdba from pg_database where datname='template0'))), "
 						   "pg_encoding_to_char(d.encoding), "
 						   "null::text AS datcollate, null::text AS datctype, 0 AS datfrozenxid, 0 AS datminmxid, "
-						   "datistemplate, '' as datacl, -1 as datconnlimit, "
+						   "datistemplate, '' as datacl, '' as rdatacl, -1 as datconnlimit, "
 						   "'pg_default' AS dattablespace "
 						   "FROM pg_database d "
 						   "WHERE datallowconn ORDER BY 1");
@@ -1359,7 +1388,7 @@ dumpCreateDB(PGconn *conn)
 						   "pg_encoding_to_char(d.encoding), "
 						   "null::text AS datcollate, null::text AS datctype, 0 AS datfrozenxid, 0 AS datminmxid, "
 						   "'f' as datistemplate, "
-						   "'' as datacl, -1 as datconnlimit, "
+						   "'' as datacl, '' as rdatacl, -1 as datconnlimit, "
 						   "'pg_default' AS dattablespace "
 						   "FROM pg_database d "
 						   "ORDER BY 1");
@@ -1376,8 +1405,9 @@ dumpCreateDB(PGconn *conn)
 		uint32		dbminmxid = atooid(PQgetvalue(res, i, 6));
 		char	   *dbistemplate = PQgetvalue(res, i, 7);
 		char	   *dbacl = PQgetvalue(res, i, 8);
-		char	   *dbconnlimit = PQgetvalue(res, i, 9);
-		char	   *dbtablespace = PQgetvalue(res, i, 10);
+		char	   *rdbacl = PQgetvalue(res, i, 9);
+		char	   *dbconnlimit = PQgetvalue(res, i, 10);
+		char	   *dbtablespace = PQgetvalue(res, i, 11);
 		char	   *fdbname;
 
 		fdbname = pg_strdup(fmtId(dbname));
@@ -1469,7 +1499,7 @@ dumpCreateDB(PGconn *conn)
 		}
 
 		if (!skip_acls &&
-			!buildACLCommands(fdbname, NULL, "DATABASE", dbacl, "", dbowner,
+			!buildACLCommands(fdbname, NULL, "DATABASE", dbacl, rdbacl, dbowner,
 							  "", server_version, buf))
 		{
 			fprintf(stderr, _("%s: could not parse ACL list (%s) for database \"%s\"\n"),
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index fd4b9e8..592a26f 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -141,6 +141,9 @@ my %pgdump_runs = (
 		dump_cmd => [
 			'pg_dumpall', '-f', "$tempdir/pg_dumpall_globals_clean.sql",
 			'-g', '-c', ], },
+	pg_dumpall_dbprivs => {
+		dump_cmd =>
+		  [ 'pg_dumpall', '-f', "$tempdir/pg_dumpall_dbprivs.sql", ], },
 	no_privs => {
 		dump_cmd =>
 		  [ 'pg_dump', '-f', "$tempdir/no_privs.sql", '-x', 'postgres', ], },
@@ -240,6 +243,7 @@ my %tests = (
 			exclude_test_table      => 1,
 			exclude_test_table_data => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_post_data       => 1,
 			test_schema_plus_blobs  => 1, },
@@ -258,6 +262,7 @@ my %tests = (
 			\QNOREPLICATION NOBYPASSRLS;\E
 			/xm,
 		like => {
+			pg_dumpall_dbprivs       => 1,
 			pg_dumpall_globals       => 1,
 			pg_dumpall_globals_clean => 1, },
 		unlike => {
@@ -292,6 +297,7 @@ my %tests = (
 			exclude_test_table_data => 1,
 			no_privs                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -310,6 +316,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -328,6 +335,7 @@ my %tests = (
 			exclude_test_table_data => 1,
 			no_privs                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -346,6 +354,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -367,6 +376,7 @@ my %tests = (
 			no_privs                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_post_data       => 1,
 			test_schema_plus_blobs  => 1, },
@@ -387,6 +397,7 @@ my %tests = (
 			no_privs                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -408,6 +419,7 @@ my %tests = (
 			no_privs                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_post_data       => 1,
 			test_schema_plus_blobs  => 1, },
@@ -427,6 +439,7 @@ my %tests = (
 			exclude_test_table_data => 1,
 			no_privs                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -445,6 +458,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -504,6 +518,7 @@ my %tests = (
 	#			exclude_test_table => 1,
 	#			exclude_test_table_data => 1,
 	#			no_privs => 1,
+	#			pg_dumpall_dbprivs       => 1,
 	#			section_data => 1,
 	#			test_schema_plus_blobs => 1,
 	#		},
@@ -529,6 +544,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -546,6 +562,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -568,6 +585,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1, },
 		unlike => {
@@ -591,6 +609,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1, },
 		unlike => {
@@ -614,6 +633,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1, },
 		unlike => {
@@ -649,6 +669,7 @@ my %tests = (
 			no_owner               => 1,
 			only_dump_test_schema  => 1,
 			only_dump_test_table   => 1,
+			pg_dumpall_dbprivs     => 1,
 			section_data           => 1,
 			test_schema_plus_blobs => 1, },
 		unlike => {
@@ -674,6 +695,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			section_data            => 1,
 			test_schema_plus_blobs  => 1, },
 		unlike => {
@@ -711,6 +733,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			section_data            => 1,
 			test_schema_plus_blobs  => 1, },
 		unlike => {
@@ -736,6 +759,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			section_data             => 1, },
 		unlike => {
 			only_dump_test_schema  => 1,
@@ -757,6 +781,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			section_data             => 1,
 			only_dump_test_schema    => 1,
 			only_dump_test_table     => 1,
@@ -777,6 +802,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			section_data             => 1,
 			only_dump_test_schema    => 1,
 			only_dump_test_table     => 1,
@@ -797,6 +823,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			section_data             => 1,
 			only_dump_test_schema    => 1,
 			only_dump_test_table     => 1,
@@ -816,6 +843,7 @@ my %tests = (
 		create_sql   => 'CREATE ROLE dump_test;',
 		regexp       => qr/^CREATE ROLE dump_test;/m,
 		like         => {
+			pg_dumpall_dbprivs       => 1,
 			pg_dumpall_globals       => 1,
 			pg_dumpall_globals_clean => 1, },
 		unlike => {
@@ -852,6 +880,33 @@ my %tests = (
 			no_owner                 => 1,
 			only_dump_test_schema    => 1,
 			only_dump_test_table     => 1,
+			pg_dumpall_dbprivs       => 1,
+			pg_dumpall_globals       => 1,
+			pg_dumpall_globals_clean => 1,
+			schema_only              => 1,
+			section_pre_data         => 1,
+			section_post_data        => 1,
+			test_schema_plus_blobs   => 1, }, },
+	'CREATE DATABASE dump_test' => {
+		create_order => 47,
+		create_sql => 'CREATE DATABASE dump_test;',
+		regexp => qr/^
+			\QCREATE DATABASE dump_test WITH TEMPLATE = template0 \E
+			.*;/xm,
+		like   => { pg_dumpall_dbprivs => 1, },
+		unlike => {
+			binary_upgrade           => 1,
+			clean                    => 1,
+			clean_if_exists          => 1,
+			createdb                 => 1,
+			defaults                 => 1,
+			exclude_dump_test_schema => 1,
+			exclude_test_table       => 1,
+			exclude_test_table_data  => 1,
+			no_privs                 => 1,
+			no_owner                 => 1,
+			only_dump_test_schema    => 1,
+			only_dump_test_table     => 1,
 			pg_dumpall_globals       => 1,
 			pg_dumpall_globals_clean => 1,
 			schema_only              => 1,
@@ -872,6 +927,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -909,6 +965,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -942,6 +999,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -974,6 +1032,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1005,6 +1064,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1036,6 +1096,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1066,6 +1127,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_post_data        => 1, },
 		unlike => {
@@ -1097,6 +1159,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_post_data       => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1126,6 +1189,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1156,6 +1220,7 @@ my %tests = (
 			no_privs                 => 1,
 			no_owner                 => 1,
 			only_dump_test_schema    => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1,
 			test_schema_plus_blobs   => 1,
@@ -1183,6 +1248,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1206,6 +1272,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1236,6 +1303,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1266,6 +1334,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1304,6 +1373,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1335,6 +1405,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1359,6 +1430,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -1383,6 +1455,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -1411,6 +1484,7 @@ my %tests = (
 #			exclude_test_table_data => 1,
 #			no_privs => 1,
 #			no_owner => 1,
+#			pg_dumpall_dbprivs       => 1,
 #			schema_only => 1,
 #			section_post_data => 1,
 #		},
@@ -1440,6 +1514,7 @@ my %tests = (
 			exclude_test_table_data => 1,
 			no_privs                => 1,
 			no_owner                => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1, },
 		unlike => {
@@ -1471,6 +1546,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1502,6 +1578,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1531,6 +1608,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_post_data       => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1559,6 +1637,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_post_data       => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1587,6 +1666,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_post_data       => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1615,6 +1695,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_post_data       => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1643,6 +1724,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_post_data       => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1667,6 +1749,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1691,6 +1774,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -1723,6 +1807,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1753,6 +1838,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1785,6 +1871,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1815,6 +1902,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -1845,6 +1933,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -1873,6 +1962,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -1901,6 +1991,7 @@ my %tests = (
 			exclude_test_table_data  => 1,
 			no_privs                 => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_post_data        => 1, },
 		unlike => {
@@ -2021,6 +2112,7 @@ my %tests = (
 			no_owner                 => 1,
 			only_dump_test_schema    => 1,
 			only_dump_test_table     => 1,
+			pg_dumpall_dbprivs       => 1,
 			pg_dumpall_globals       => 1,
 			schema_only              => 1, }, },
 	'GRANT USAGE ON SCHEMA dump_test_second_schema' => {
@@ -2040,6 +2132,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -2047,6 +2140,30 @@ my %tests = (
 			only_dump_test_table   => 1,
 			pg_dumpall_globals     => 1,
 			test_schema_plus_blobs => 1, }, },
+	'GRANT CREATE ON DATABASE dump_test' => {
+		create_order => 48,
+		create_sql   => 'GRANT CREATE ON DATABASE dump_test TO dump_test;',
+		regexp => qr/^
+			\QGRANT CREATE ON DATABASE dump_test TO dump_test;\E
+			/xm,
+		like => {
+			pg_dumpall_dbprivs       => 1, },
+		unlike => {
+			binary_upgrade           => 1,
+			clean                    => 1,
+			clean_if_exists          => 1,
+			createdb                 => 1,
+			defaults                 => 1,
+			exclude_dump_test_schema => 1,
+			exclude_test_table       => 1,
+			exclude_test_table_data  => 1,
+			no_owner                 => 1,
+			only_dump_test_schema    => 1,
+			only_dump_test_table     => 1,
+			pg_dumpall_globals       => 1,
+			schema_only              => 1,
+			section_pre_data         => 1, ,
+			test_schema_plus_blobs   => 1, }, },
 	'GRANT SELECT ON TABLE test_table' => {
 		create_order => 5,
 		create_sql   => 'GRANT SELECT ON TABLE dump_test.test_table
@@ -2062,6 +2179,7 @@ my %tests = (
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
 			only_dump_test_table    => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -2085,6 +2203,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -2110,6 +2229,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -2135,6 +2255,7 @@ my %tests = (
 			exclude_test_table_data => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			schema_only             => 1,
 			section_pre_data        => 1,
 			test_schema_plus_blobs  => 1, },
@@ -2159,6 +2280,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -2242,6 +2364,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -2268,6 +2391,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			test_schema_plus_blobs  => 1,
 			section_post_data       => 1, },
 		unlike => {
@@ -2293,6 +2417,7 @@ my %tests = (
 			no_privs                => 1,
 			no_owner                => 1,
 			only_dump_test_schema   => 1,
+			pg_dumpall_dbprivs      => 1,
 			test_schema_plus_blobs  => 1,
 			section_post_data       => 1, },
 		unlike => {
@@ -2302,6 +2427,30 @@ my %tests = (
 			pg_dumpall_globals       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, }, },
+	'REVOKE CONNECT ON DATABASE dump_test FROM public' => {
+		create_order => 49,
+		create_sql   => 'REVOKE CONNECT ON DATABASE dump_test FROM public;',
+		regexp => qr/^
+			\QREVOKE CONNECT,TEMPORARY ON DATABASE dump_test FROM PUBLIC;\E\n
+			\QGRANT TEMPORARY ON DATABASE dump_test TO PUBLIC;\E
+			/xm,
+		like => {
+			pg_dumpall_dbprivs       => 1, },
+		unlike => {
+			binary_upgrade           => 1,
+			clean                    => 1,
+			clean_if_exists          => 1,
+			createdb                 => 1,
+			defaults                 => 1,
+			exclude_dump_test_schema => 1,
+			exclude_test_table       => 1,
+			exclude_test_table_data  => 1,
+			no_owner                 => 1,
+			only_dump_test_schema    => 1,
+			only_dump_test_table     => 1,
+			schema_only              => 1,
+			section_pre_data         => 1,
+			test_schema_plus_blobs   => 1, }, },
 	'REVOKE EXECUTE ON FUNCTION pg_sleep() FROM public' => {
 		create_order => 15,
 		create_sql   => 'REVOKE EXECUTE ON FUNCTION pg_sleep(float8)
@@ -2319,6 +2468,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -2339,6 +2489,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -2362,6 +2513,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
@@ -2382,6 +2534,7 @@ my %tests = (
 			exclude_test_table       => 1,
 			exclude_test_table_data  => 1,
 			no_owner                 => 1,
+			pg_dumpall_dbprivs       => 1,
 			schema_only              => 1,
 			section_pre_data         => 1, },
 		unlike => {
-- 
2.7.4

#11Michael Paquier
michael.paquier@gmail.com
In reply to: Stephen Frost (#10)
Re: dumping database privileges broken in 9.6

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Noah Misch
noah@leadboat.com
In reply to: Stephen Frost (#9)
Re: dumping database privileges broken in 9.6

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:
/messages/by-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.)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13Stephen Frost
sfrost@snowman.net
In reply to: Noah Misch (#12)
Re: dumping database privileges broken in 9.6

* 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:
/messages/by-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

#14Noah Misch
noah@leadboat.com
In reply to: Stephen Frost (#13)
Re: dumping database privileges broken in 9.6

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:
/messages/by-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?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15Michael Paquier
michael.paquier@gmail.com
In reply to: Stephen Frost (#13)
Re: dumping database privileges broken in 9.6

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16Stephen Frost
sfrost@snowman.net
In reply to: Noah Misch (#14)
Re: dumping database privileges broken in 9.6

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 <javascript:;>) wrote:

On Sat, Jul 09, 2016 at 12:55:33AM -0400, Stephen Frost wrote:

* Noah Misch (noah@leadboat.com <javascript:;>) 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:

/messages/by-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

#17Stephen Frost
sfrost@snowman.net
In reply to: Michael Paquier (#15)
Re: dumping database privileges broken in 9.6

* 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

#18Stephen Frost
sfrost@snowman.net
In reply to: Noah Misch (#14)
Re: dumping database privileges broken in 9.6

* 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:
/messages/by-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