Tom, all, * Stephen Frost (sfr...@snowman.net) wrote: > Alright, here's an updated patch which cleans things up a bit and adds > comments to explain what's going on. I also updated the comments in > acl.h to explain that ordering actually does matter.
Getting back to this, here's rebased patches for master/v10 and 9.6 (which only had whitespace differences, probably pgindent to blame there..). I'm going to push these later today unless there's other comments on it. Thanks! Stephen
From ac86eb5451492bcb72f25cceab4ae467716ea073 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
Date: Thu, 3 Aug 2017 21:23:37 -0400
Subject: [PATCH] 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 | 51 ++++++++++++++++++++++++++++++++-------------
src/include/utils/acl.h | 14 ++++++++++---
2 files changed, 47 insertions(+), 18 deletions(-)
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index dfc611848b..e4c95feb63 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/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 "
- "(SELECT pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) AS acl "
- "EXCEPT "
- "SELECT pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s)))) as foo)",
+ 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))) "
+ "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 "
- "(SELECT pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s))) AS acl "
- "EXCEPT "
- "SELECT pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s)))) as foo)",
+ printfPQExpBuffer(racl_subquery,
+ "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM "
+ "(SELECT acl, row_n FROM "
+ "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.unnest(pip.initprivs) AS acl "
- "EXCEPT "
- "SELECT pg_catalog.unnest(pg_catalog.acldefault(%s,%s))) as foo) END",
+ "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM "
+ "(SELECT acl, row_n FROM pg_catalog.unnest(pip.initprivs) "
+ "WITH ORDINALITY AS initp(acl,row_n) "
+ "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 "
- "EXCEPT "
- "SELECT pg_catalog.unnest(pip.initprivs)) as foo) END",
+ "(SELECT acl, row_n FROM "
+ "pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) "
+ "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);
}
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 43273eaab5..254a811aff 100644
--- a/src/include/utils/acl.h
+++ b/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
- * ordering of the items, but we do expect that there are no two entries
- * in the array with the same grantor and grantee.
+ * array represents "no privileges".
+ *
+ * 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
--
2.11.0
From 73523ccf2c1701797d47ee1c608ccd29c233a6dc Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
Date: Thu, 3 Aug 2017 21:23:37 -0400
Subject: [PATCH] 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 | 51 ++++++++++++++++++++++++++++++++-------------
src/include/utils/acl.h | 14 ++++++++++---
2 files changed, 47 insertions(+), 18 deletions(-)
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 11870a33fa..c171978060 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -729,21 +729,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 "
- "(SELECT pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) AS acl "
- "EXCEPT "
- "SELECT pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s)))) as foo)",
+ 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))) "
+ "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 "
- "(SELECT pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s))) AS acl "
- "EXCEPT "
- "SELECT pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s)))) as foo)",
+ printfPQExpBuffer(racl_subquery,
+ "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM "
+ "(SELECT acl, row_n FROM "
+ "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,
@@ -768,19 +783,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.unnest(pip.initprivs) AS acl "
- "EXCEPT "
- "SELECT pg_catalog.unnest(pg_catalog.acldefault(%s,%s))) as foo) END",
+ "(SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM "
+ "(SELECT acl, row_n FROM pg_catalog.unnest(pip.initprivs) "
+ "WITH ORDINALITY AS initp(acl,row_n) "
+ "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 "
- "EXCEPT "
- "SELECT pg_catalog.unnest(pip.initprivs)) as foo) END",
+ "(SELECT acl, row_n FROM "
+ "pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) "
+ "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);
}
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 5a4f1714f3..14279f79da 100644
--- a/src/include/utils/acl.h
+++ b/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
- * ordering of the items, but we do expect that there are no two entries
- * in the array with the same grantor and grantee.
+ * array represents "no privileges".
+ *
+ * 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
--
2.11.0
signature.asc
Description: Digital signature
