ACL dump ordering broken as well for tablespaces

[Michael Paquier]

Hi all,

As some may have noticed, I have been looking at the ACL dump ordering
for databases, and I have noticed the same issue with tablespaces:
https://www.postgresql.org/message-id/20190522062626.gc1...@paquier.xyz

For the sake of avoiding looking at the other email, here is how to
reproduce the issue:
1) First issue those SQLs:
\! rm -rf /tmp/tbspc/
\! mkdir -p /tmp/tbspc/
CREATE ROLE a_user;
CREATE ROLE b_user WITH SUPERUSER;
CREATE ROLE c_user;
CREATE TABLESPACE poo LOCATION '/tmp/tbspc/';
SET SESSION AUTHORIZATION b_user;
REVOKE ALL ON TABLESPACE poo FROM public;
GRANT CREATE ON TABLESPACE poo TO c_user WITH GRANT OPTION;
SET SESSION AUTHORIZATION c_user;
GRANT CREATE ON TABLESPACE poo TO a_user
2) Use pg_dumpall -g, where you would notice the following set of
GRANT queries:
CREATE TABLESPACE poo OWNER postgres LOCATION '/tmp/tbspc';
SET SESSION AUTHORIZATION c_user;
GRANT ALL ON TABLESPACE poo TO a_user;
RESET SESSION AUTHORIZATION;
GRANT ALL ON TABLESPACE poo TO c_user WITH GRANT OPTION;
3) Trying to restore results in a failure for the first GRANT query,
as the second one has not set yet the authorizations for c_user.

Attached is a patch to fix that, so as pg_dumpall does not complain
when piling up GRANT commands using WITH GRANT OPTION.  Are there any
complains to apply that down to 9.6?

When applying the patch, the set of GRANT queries is reordered:
 CREATE TABLESPACE poo OWNER postgres LOCATION '/tmp/tbspc';
+GRANT ALL ON TABLESPACE poo TO c_user WITH GRANT OPTION;
 SET SESSION AUTHORIZATION c_user;
 GRANT ALL ON TABLESPACE poo TO a_user;
 RESET SESSION AUTHORIZATION;
-GRANT ALL ON TABLESPACE poo TO c_user WITH GRANT OPTION;

As the problem is kind of different than the database case, I wanted
to spawn anyway a new thread, but I got a bonus question: what would
it take to support pg_init_privs for databases and tablespaces?  If we
could get that to work, then all the ACL-related queries built for all
objects could make use of buildACLQueries(), which would avoid extra
diffs in the dump code for dbs and tbspaces.

Thoughts?
--
Michael

diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 102731ea0c..e833db4533 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1166,19 +1166,38 @@ dumpTablespaces(PGconn *conn)
 	 *
 	 * See buildACLQueries() and buildACLCommands().
 	 *
+	 * The order in which privileges are in the ACL string (the order they
+	 * have been GRANT'd in, which the backend maintains) must be preserved to
+	 * ensure that GRANTs WITH GRANT OPTION and subsequent GRANTs based on
+	 * those are dumped in the correct order.
+	 *
 	 * Note that we do not support initial privileges (pg_init_privs) on
-	 * tablespaces.
+	 * tablespaces, so this logic cannot make use of buildACLQueries().
 	 */
 	if (server_version >= 90600)
 		res = executeQuery(conn, "SELECT oid, spcname, "
 						   "pg_catalog.pg_get_userbyid(spcowner) AS spcowner, "
 						   "pg_catalog.pg_tablespace_location(oid), "
-						   "(SELECT pg_catalog.array_agg(acl) FROM (SELECT pg_catalog.unnest(coalesce(spcacl,pg_catalog.acldefault('t',spcowner))) AS acl "
-						   "EXCEPT SELECT pg_catalog.unnest(pg_catalog.acldefault('t',spcowner))) as foo)"
-						   "AS spcacl,"
-						   "(SELECT pg_catalog.array_agg(acl) FROM (SELECT pg_catalog.unnest(pg_catalog.acldefault('t',spcowner)) AS acl "
-						   "EXCEPT SELECT pg_catalog.unnest(coalesce(spcacl,pg_catalog.acldefault('t',spcowner)))) as foo)"
-						   "AS rspcacl,"
+						   "(SELECT array_agg(acl ORDER BY row_n) FROM "
+						   "  (SELECT acl, row_n FROM "
+						   "     unnest(coalesce(spcacl,acldefault('t',spcowner))) "
+						   "     WITH ORDINALITY AS perm(acl,row_n) "
+						   "   WHERE NOT EXISTS ( "
+						   "     SELECT 1 "
+						   "     FROM unnest(acldefault('t',spcowner)) "
+						   "       AS init(init_acl) "
+						   "     WHERE acl = init_acl)) AS spcacls) "
+						   " AS spcacl, "
+						   "(SELECT array_agg(acl ORDER BY row_n) FROM "
+						   "  (SELECT acl, row_n FROM "
+						   "     unnest(acldefault('t',spcowner)) "
+						   "     WITH ORDINALITY AS initp(acl,row_n) "
+						   "   WHERE NOT EXISTS ( "
+						   "     SELECT 1 "
+						   "     FROM unnest(coalesce(spcacl,acldefault('t',spcowner))) "
+						   "       AS permp(orig_acl) "
+						   "     WHERE acl = orig_acl)) AS rspcacls) "
+						   " AS rspcacl, "
 						   "array_to_string(spcoptions, ', '),"
 						   "pg_catalog.shobj_description(oid, 'pg_tablespace') "
 						   "FROM pg_catalog.pg_tablespace "

signature.asc
Description: PGP signature