Fix ordering in pg_dump of GRANTs

The order in which GRANTs are output is important as GRANTs which have
been GRANT'd by individuals via WITH GRANT OPTION GRANTs have to come
after the GRANT which included the WITH GRANT OPTION.  This happens
naturally in the backend during normal operation as we only change
existing ACLs in-place, only add new ACLs to the end, and when removing
an ACL we remove any which depend on it also.

Also, adjust the comments in acl.h to make this clear.

Unfortunately, the updates to pg_dump to handle initial privileges
involved pulling apart ACLs and then combining them back together and
could end up putting them back together in an invalid order, leading to
dumps which wouldn't restore.

Fix this by adjusting the queries used by pg_dump to ensure that the
ACLs are rebuilt in the same order in which they were originally.

Back-patch to 9.6 where the changes for initial privileges were done.

src/bin/pg_dump/dumputils.c

@@ -722,21 +722,36 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
 	 * We always perform this delta on all ACLs and expect that by the time
 	 * these are run the initial privileges will be in place, even in a binary
 	 * upgrade situation (see below).
+	 *
+	 * Finally, the order in which privileges are in the ACL string (the order
+	 * they 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.
 	 */
-	printfPQExpBuffer(acl_subquery, "(SELECT pg_catalog.array_agg(acl) FROM "
+	printfPQExpBuffer(acl_subquery,
-					  "(SELECT pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) AS acl "
+					  "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM "
-					  "EXCEPT "
+					  "(SELECT acl, row_n FROM "
-					  "SELECT pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s)))) as foo)",
+					  "pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) "
+					  "WITH ORDINALITY AS perm(acl,row_n) "
+					  "WHERE NOT EXISTS ( "
+					  "SELECT 1 FROM "
+					  "pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s))) "
+					  "AS init(init_acl) WHERE acl = init_acl)) as foo)",
 					  acl_column,
 					  obj_kind,
 					  acl_owner,
 					  obj_kind,
 					  acl_owner);
 
-	printfPQExpBuffer(racl_subquery, "(SELECT pg_catalog.array_agg(acl) FROM "
+	printfPQExpBuffer(racl_subquery,
-					  "(SELECT pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s))) AS acl "
+					  "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM "
-					  "EXCEPT "
+					  "(SELECT acl, row_n FROM "
-					  "SELECT pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s)))) as foo)",
+					  "pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%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))) "
+					  "AS permp(orig_acl) WHERE acl = orig_acl)) as foo)",
 					  obj_kind,
 					  acl_owner,
 					  acl_column,
@@ -761,19 +776,25 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
 	{
 		printfPQExpBuffer(init_acl_subquery,
 						  "CASE WHEN privtype = 'e' THEN "
-						  "(SELECT pg_catalog.array_agg(acl) FROM "
+						  "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM "
-						  "(SELECT pg_catalog.unnest(pip.initprivs) AS acl "
+						  "(SELECT acl, row_n FROM pg_catalog.unnest(pip.initprivs) "
-						  "EXCEPT "
+						  "WITH ORDINALITY AS initp(acl,row_n) "
-						  "SELECT pg_catalog.unnest(pg_catalog.acldefault(%s,%s))) as foo) END",
+						  "WHERE NOT EXISTS ( "
+						  "SELECT 1 FROM "
+						  "pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) "
+						  "AS privm(orig_acl) WHERE acl = orig_acl)) as foo) END",
 						  obj_kind,
 						  acl_owner);
 
 		printfPQExpBuffer(init_racl_subquery,
 						  "CASE WHEN privtype = 'e' THEN "
 						  "(SELECT pg_catalog.array_agg(acl) FROM "
-						  "(SELECT pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) AS acl "
+						  "(SELECT acl, row_n FROM "
-						  "EXCEPT "
+						  "pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) "
-						  "SELECT pg_catalog.unnest(pip.initprivs)) as foo) END",
+						  "WITH ORDINALITY AS privp(acl,row_n) "
+						  "WHERE NOT EXISTS ( "
+						  "SELECT 1 FROM pg_catalog.unnest(pip.initprivs) "
+						  "AS initp(init_acl) WHERE acl = init_acl)) as foo) END",
 						  obj_kind,
 						  acl_owner);
 	}

src/include/utils/acl.h

@@ -12,9 +12,17 @@
  * NOTES
  *	  An ACL array is simply an array of AclItems, representing the union
  *	  of the privileges represented by the individual items.  A zero-length
- *	  array represents "no privileges".  There are no assumptions about the
+ *	  array represents "no privileges".
- *	  ordering of the items, but we do expect that there are no two entries
+ *
- *	  in the array with the same grantor and grantee.
+ *	  The order of items in the array is important as client utilities (in
+ *	  particular, pg_dump, though possibly other clients) expect to be able
+ *	  to issue GRANTs in the ordering of the items in the array.  The reason
+ *	  this matters is that GRANTs WITH GRANT OPTION must be before any GRANTs
+ *	  which depend on it.  This happens naturally in the backend during
+ *	  operations as we update ACLs in-place, new items are appended, and
+ *	  existing entries are only removed if there's no dependency on them (no
+ *	  GRANT can been based on it, or, if there was, those GRANTs are also
+ *	  removed).
  *
  *	  For backward-compatibility purposes we have to allow null ACL entries
  *	  in system catalogs.  A null ACL will be treated as meaning "default