pg_dump handling of ALTER DEFAULT PRIVILEGES IN SCHEMA

From cdffe044438135279515277797e5835997799317 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Mon, 6 Sep 2021 05:32:14 +0000
Subject: [PATCH v2 1/1] Fix pg_dump handling of ALTER DEFAULT PRIVILEGES IN
 SCHEMA.

When dumping ACLs, pg_dump uses the hard-wired default privileges
(i.e., acldefault()) as a reference.  While this is usually okay,
it sometimes doesn't work for schema-local default ACLs (i.e.,
pg_default_acl entries with defaclnamespace != 0).  This is because
the default for a schema-local default ACL entry is actually an
empty ACL, as per the following note in aclchk.c:

	/*
	 * The default for a global entry is the hard-wired default ACL for the
	 * particular object type.  The default for non-global entries is an empty
	 * ACL.  This must be so because global entries replace the hard-wired
	 * defaults, while others are added on.
	 */

To fix, adjust the pg_dump query for dumping ACLs to use NULL
instead of acldefault() for schema-local default ACLs.
---
 src/bin/pg_dump/dumputils.c      | 56 ++++++++++++++++++++++++++--------------
 src/bin/pg_dump/dumputils.h      |  3 ++-
 src/bin/pg_dump/pg_dump.c        | 25 +++++++++---------
 src/bin/pg_dump/t/002_pg_dump.pl | 19 ++++++++++++++
 4 files changed, 71 insertions(+), 32 deletions(-)

diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index ea67e52a3f..a6799bb5a5 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -726,8 +726,30 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
 				PQExpBuffer init_acl_subquery, PQExpBuffer init_racl_subquery,
 				const char *acl_column, const char *acl_owner,
 				const char *initprivs_expr,
-				const char *obj_kind, bool binary_upgrade)
+				const char *obj_kind, bool binary_upgrade, bool is_default_acl)
 {
+	PQExpBuffer	acldefault_expr = createPQExpBuffer();
+
+	/*
+	 * Build the expression for obtaining the default privileges.
+	 *
+	 * This is ordinarily just acldefault(), but for schema-local default ACLs
+	 * (i.e., entries in pg_default_acl with defaclnamespace != 0), the default
+	 * is actually an empty ACL.
+	 */
+	if (is_default_acl)
+		printfPQExpBuffer(acldefault_expr,
+						  "CASE WHEN defaclnamespace = 0 THEN "
+						  "pg_catalog.acldefault(%s,%s) "
+						  "ELSE NULL END",
+						  obj_kind,
+						  acl_owner);
+	else
+		printfPQExpBuffer(acldefault_expr,
+						  "pg_catalog.acldefault(%s,%s)",
+						  obj_kind,
+						  acl_owner);
+
 	/*
 	 * To get the delta from what the permissions were at creation time
 	 * (either initdb or CREATE EXTENSION) vs. what they are now, we have to
@@ -762,34 +784,30 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
 	printfPQExpBuffer(acl_subquery,
 					  "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM "
 					  "(SELECT acl, row_n FROM "
-					  "pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) "
+					  "pg_catalog.unnest(coalesce(%s,%s)) "
 					  "WITH ORDINALITY AS perm(acl,row_n) "
 					  "WHERE NOT EXISTS ( "
 					  "SELECT 1 FROM "
-					  "pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) "
+					  "pg_catalog.unnest(coalesce(%s,%s)) "
 					  "AS init(init_acl) WHERE acl = init_acl)) as foo)",
 					  acl_column,
-					  obj_kind,
-					  acl_owner,
+					  acldefault_expr->data,
 					  initprivs_expr,
-					  obj_kind,
-					  acl_owner);
+					  acldefault_expr->data);
 
 	printfPQExpBuffer(racl_subquery,
 					  "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM "
 					  "(SELECT acl, row_n FROM "
-					  "pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) "
+					  "pg_catalog.unnest(coalesce(%s,%s)) "
 					  "WITH ORDINALITY AS initp(acl,row_n) "
 					  "WHERE NOT EXISTS ( "
 					  "SELECT 1 FROM "
-					  "pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) "
+					  "pg_catalog.unnest(coalesce(%s,%s)) "
 					  "AS permp(orig_acl) WHERE acl = orig_acl)) as foo)",
 					  initprivs_expr,
-					  obj_kind,
-					  acl_owner,
+					  acldefault_expr->data,
 					  acl_column,
-					  obj_kind,
-					  acl_owner);
+					  acldefault_expr->data);
 
 	/*
 	 * In binary upgrade mode we don't run the extension script but instead
@@ -814,23 +832,21 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
 						  "WITH ORDINALITY AS initp(acl,row_n) "
 						  "WHERE NOT EXISTS ( "
 						  "SELECT 1 FROM "
-						  "pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) "
+						  "pg_catalog.unnest(%s) "
 						  "AS privm(orig_acl) WHERE acl = orig_acl)) as foo) END",
 						  initprivs_expr,
-						  obj_kind,
-						  acl_owner);
+						  acldefault_expr->data);
 
 		printfPQExpBuffer(init_racl_subquery,
 						  "CASE WHEN privtype = 'e' THEN "
 						  "(SELECT pg_catalog.array_agg(acl) FROM "
 						  "(SELECT acl, row_n FROM "
-						  "pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) "
+						  "pg_catalog.unnest(%s) "
 						  "WITH ORDINALITY AS privp(acl,row_n) "
 						  "WHERE NOT EXISTS ( "
 						  "SELECT 1 FROM pg_catalog.unnest(%s) "
 						  "AS initp(init_acl) WHERE acl = init_acl)) as foo) END",
-						  obj_kind,
-						  acl_owner,
+						  acldefault_expr->data,
 						  initprivs_expr);
 	}
 	else
@@ -838,6 +854,8 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
 		printfPQExpBuffer(init_acl_subquery, "NULL");
 		printfPQExpBuffer(init_racl_subquery, "NULL");
 	}
+
+	destroyPQExpBuffer(acldefault_expr);
 }
 
 /*
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index f5465f19ae..037f4b03bb 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -55,7 +55,8 @@ extern void buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
 							PQExpBuffer init_acl_subquery, PQExpBuffer init_racl_subquery,
 							const char *acl_column, const char *acl_owner,
 							const char *initprivs_expr,
-							const char *obj_kind, bool binary_upgrade);
+							const char *obj_kind, bool binary_upgrade,
+							bool is_default_acl);
 
 extern bool variable_is_guc_list_quote(const char *name);
 
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 67be849829..840d5a7a7a 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3421,7 +3421,7 @@ getBlobs(Archive *fout)
 
 		buildACLQueries(acl_subquery, racl_subquery, init_acl_subquery,
 						init_racl_subquery, "l.lomacl", "l.lomowner",
-						"pip.initprivs", "'L'", dopt->binary_upgrade);
+						"pip.initprivs", "'L'", dopt->binary_upgrade, false);
 
 		appendPQExpBuffer(blobQry,
 						  "SELECT l.oid, (%s l.lomowner) AS rolname, "
@@ -4866,7 +4866,7 @@ getNamespaces(Archive *fout, int *numNamespaces)
 						"         n.nspowner::regrole, n.nspowner::regrole),"
 						"  format('=UC/%s', n.nspowner::regrole)]::aclitem[] "
 						"ELSE pip.initprivs END",
-						"'n'", dopt->binary_upgrade);
+						"'n'", dopt->binary_upgrade, false);
 
 		appendPQExpBuffer(query, "SELECT n.tableoid, n.oid, n.nspname, "
 						  "n.nspowner, "
@@ -5117,7 +5117,7 @@ getTypes(Archive *fout, int *numTypes)
 
 		buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
 						initracl_subquery, "t.typacl", "t.typowner",
-						"pip.initprivs", "'T'", dopt->binary_upgrade);
+						"pip.initprivs", "'T'", dopt->binary_upgrade, false);
 
 		appendPQExpBuffer(query, "SELECT t.tableoid, t.oid, t.typname, "
 						  "t.typnamespace, "
@@ -5820,7 +5820,7 @@ getAggregates(Archive *fout, int *numAggs)
 
 		buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
 						initracl_subquery, "p.proacl", "p.proowner",
-						"pip.initprivs", "'f'", dopt->binary_upgrade);
+						"pip.initprivs", "'f'", dopt->binary_upgrade, false);
 
 		agg_check = (fout->remoteVersion >= 110000 ? "p.prokind = 'a'"
 					 : "p.proisagg");
@@ -6033,7 +6033,7 @@ getFuncs(Archive *fout, int *numFuncs)
 
 		buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
 						initracl_subquery, "p.proacl", "p.proowner",
-						"pip.initprivs", "'f'", dopt->binary_upgrade);
+						"pip.initprivs", "'f'", dopt->binary_upgrade, false);
 
 		not_agg_check = (fout->remoteVersion >= 110000 ? "p.prokind <> 'a'"
 						 : "NOT p.proisagg");
@@ -6332,11 +6332,11 @@ getTables(Archive *fout, int *numTables)
 						"pip.initprivs",
 						"CASE WHEN c.relkind = " CppAsString2(RELKIND_SEQUENCE)
 						" THEN 's' ELSE 'r' END::\"char\"",
-						dopt->binary_upgrade);
+						dopt->binary_upgrade, false);
 
 		buildACLQueries(attacl_subquery, attracl_subquery, attinitacl_subquery,
 						attinitracl_subquery, "at.attacl", "c.relowner",
-						"pip.initprivs", "'c'", dopt->binary_upgrade);
+						"pip.initprivs", "'c'", dopt->binary_upgrade, false);
 
 		appendPQExpBuffer(query,
 						  "SELECT c.tableoid, c.oid, c.relname, "
@@ -8311,7 +8311,7 @@ getProcLangs(Archive *fout, int *numProcLangs)
 
 		buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
 						initracl_subquery, "l.lanacl", "l.lanowner",
-						"pip.initprivs", "'l'", dopt->binary_upgrade);
+						"pip.initprivs", "'l'", dopt->binary_upgrade, false);
 
 		/* pg_language has a laninline column */
 		appendPQExpBuffer(query, "SELECT l.tableoid, l.oid, "
@@ -9543,7 +9543,7 @@ getForeignDataWrappers(Archive *fout, int *numForeignDataWrappers)
 
 		buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
 						initracl_subquery, "f.fdwacl", "f.fdwowner",
-						"pip.initprivs", "'F'", dopt->binary_upgrade);
+						"pip.initprivs", "'F'", dopt->binary_upgrade, false);
 
 		appendPQExpBuffer(query, "SELECT f.tableoid, f.oid, f.fdwname, "
 						  "(%s f.fdwowner) AS rolname, "
@@ -9710,7 +9710,7 @@ getForeignServers(Archive *fout, int *numForeignServers)
 
 		buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
 						initracl_subquery, "f.srvacl", "f.srvowner",
-						"pip.initprivs", "'S'", dopt->binary_upgrade);
+						"pip.initprivs", "'S'", dopt->binary_upgrade, false);
 
 		appendPQExpBuffer(query, "SELECT f.tableoid, f.oid, f.srvname, "
 						  "(%s f.srvowner) AS rolname, "
@@ -9858,7 +9858,7 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
 						initracl_subquery, "defaclacl", "defaclrole",
 						"pip.initprivs",
 						"CASE WHEN defaclobjtype = 'S' THEN 's' ELSE defaclobjtype END::\"char\"",
-						dopt->binary_upgrade);
+						dopt->binary_upgrade, true);
 
 		appendPQExpBuffer(query, "SELECT d.oid, d.tableoid, "
 						  "(%s d.defaclrole) AS defaclrole, "
@@ -15730,7 +15730,8 @@ dumpTable(Archive *fout, const TableInfo *tbinfo)
 
 			buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
 							initracl_subquery, "at.attacl", "c.relowner",
-							"pip.initprivs", "'c'", dopt->binary_upgrade);
+							"pip.initprivs", "'c'", dopt->binary_upgrade,
+							false);
 
 			appendPQExpBuffer(query,
 							  "SELECT at.attname, "
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index e1b7e31458..6662d0b92b 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -475,6 +475,25 @@ my %tests = (
 		unlike => { no_privs => 1, },
 	  },
 
+	'ALTER DEFAULT PRIVILEGES FOR ROLE ... IN SCHEMA ...'
+	  => {
+		create_order => 57,
+		create_sql   => 'ALTER DEFAULT PRIVILEGES
+					   FOR ROLE regress_dump_test_role IN SCHEMA dump_test
+					   GRANT ALL ON FUNCTIONS TO PUBLIC;',
+		regexp => qr/^
+			\QALTER DEFAULT PRIVILEGES \E
+			\QFOR ROLE regress_dump_test_role IN SCHEMA dump_test \E
+			\QGRANT ALL ON FUNCTIONS  TO PUBLIC;\E
+			/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
+		unlike => {
+			exclude_dump_test_schema => 1,
+			no_privs                 => 1,
+		},
+	  },
+
 	'ALTER ROLE regress_dump_test_role' => {
 		regexp => qr/^
 			\QALTER ROLE regress_dump_test_role WITH \E
-- 
2.16.6