Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost <sfr...@snowman.net> writes: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> AFAICT, pg_dump has no notion that it needs to be careful about the order > >> in which permissions are granted. I did > > > I'm afraid that's correct, though I believe that's always been the case. > > I spent some time looking into this today and from what I've gathered so > > far, there's essentially an implicit (or at least, I couldn't find any > > explicit reference to it) ordering in ACLs whereby a role which was > > given a GRANT OPTION always appears in the ACL list before an ACL entry > > where that role is GRANT'ing that option to another role, and this is > > what pg_dump was (again, implicitly, it seems) depending on to get this > > correct in prior versions. > > Yeah, I suspected that was what made it work before. I think the ordering > just comes from the fact that new ACLs are added to the end, and you can't > add an entry as a non-owner unless there's a grant WGO there already.
Right. > I suspect it would be easier to modify the backend code that munges ACL > lists so that it takes care to preserve that property, if we ever find > there are cases where it does not. It seems plausible to me that > pg_dump is not the only code that depends on that ordering. Hm, well, if we're alright with that then I think the attached would address the pg_dump issue, and I believe this would work as this code is only run for 9.6+ servers anyway. This needs more cleanup, testing, and comments explaining why we're doing this (and then perhaps comments, somewhere.. in the backend ACL code that explains that the ordering needs to be preserved), but the basic idea seems sound to me and the case you presented does work with this patch (for me, at least) whereas what's in master didn't. Thoughts? Thanks! Stephen
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
new file mode 100644
index dfc6118..8686ed0
*** a/src/bin/pg_dump/dumputils.c
--- b/src/bin/pg_dump/dumputils.c
*************** buildACLQueries(PQExpBuffer acl_subquery
*** 723,742 ****
* these are run the initial privileges will be in place, even in a binary
* upgrade situation (see below).
*/
! 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)",
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)",
obj_kind,
acl_owner,
acl_column,
--- 723,742 ----
* these are run the initial privileges will be in place, even in a binary
* upgrade situation (see below).
*/
! 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 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,
*************** buildACLQueries(PQExpBuffer acl_subquery
*** 761,779 ****
{
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",
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",
obj_kind,
acl_owner);
}
--- 761,779 ----
{
printfPQExpBuffer(init_acl_subquery,
"CASE WHEN privtype = 'e' THEN "
! "(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 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);
}
signature.asc
Description: Digital signature
