From: Michael Paquier <michael@paquier.xyz>
To: Bossart, Nathan <bossartn@amazon.com>
Cc: Haribabu Kommi <kommi.haribabu@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #15788: 'pg_dump --create' orders database GRANTs incorrectly
Date: Mon, 20 May 2019 17:27:05 +0900
Message-ID: <20190520082705.GA1921@paquier.xyz> (raw)
In-Reply-To: <0AF6E4BC-EE9A-4158-85BE-3EAE4308D228@amazon.com>
References: <15788-4e18847520ebcc75@postgresql.org>
<9C9271CF-9A7A-45AE-B034-1246A93788C9@amazon.com>
<CAJrrPGcsS_=bTuKvPOg6TEgkrc9PaEJVCFmhm=z_7VKpLnjxOg@mail.gmail.com>
<4D066CB2-B177-40B4-8510-9318B9A184CE@amazon.com>
<20190504123221.GA2127@paquier.xyz>
<0AF6E4BC-EE9A-4158-85BE-3EAE4308D228@amazon.com>
On Mon, May 06, 2019 at 04:08:47PM +0000, Bossart, Nathan wrote:
> Added here: https://commitfest.postgresql.org/23/2111/
Thanks for adding the patch to the CF.
With your patch attached the difference in the dump is plain:
REVOKE CONNECT,TEMPORARY ON DATABASE mydb FROM PUBLIC;
+GRANT TEMPORARY ON DATABASE mydb TO c_user WITH GRANT OPTION;
SET SESSION AUTHORIZATION c_user;
GRANT TEMPORARY ON DATABASE mydb TO a_user;
RESET SESSION AUTHORIZATION;
-GRANT TEMPORARY ON DATABASE mydb TO c_user WITH GRANT OPTION;
So what happens is that the GRANT command to a_user fails when
switching to the session context to c_user as this user does not have
yet the authorization to perform this command. If the GRANT
permissions assigned to c_user are moved prior its actual actions then
the restore is able to work. I have been looking at it, and wondered
first if we could have just used buildACLQueries(), until I noticed
that we don't support initial privileges on databases, so the patch
you have sent looks fine to me.
I had first a hard time parsing the subqueries added, so I have
tweaked your patch with more indentation, and a comment block with
more details about why we need to preserve the ACL ordering (you will
note that I don't have a lot of imagination here).
v12 beta1 is going to ship soon, so let's wait for the version to be
tagged before committing it.
--
Michael
Attachments:
[text/x-diff] database-acls-v3.patch (2.5K, ../20190520082705.GA1921@paquier.xyz/2-database-acls-v3.patch)
download | inline diff:diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.cindex a8f1ad7333..8931fc4014 100644--- a/src/bin/pg_dump/pg_dump.c+++ b/src/bin/pg_dump/pg_dump.c@@ -2664,20 +2664,41 @@ dumpDatabase(Archive *fout)
pg_log_info("saving database definition");
- /* Fetch the database-level properties for this database */+ /*+ * Fetch the database-level properties for this database.+ *+ * 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 initial privileges+ * (pg_init_privs) are not supported on databases, so this logic cannot+ * make use of buildACLQueries().+ */
if (fout->remoteVersion >= 90600)
{
appendPQExpBuffer(dbQry, "SELECT tableoid, oid, datname, "
"(%s datdba) AS dba, "
"pg_encoding_to_char(encoding) AS encoding, "
"datcollate, datctype, datfrozenxid, datminmxid, "
- "(SELECT array_agg(acl ORDER BY acl::text COLLATE \"C\") FROM ( "- " SELECT unnest(coalesce(datacl,acldefault('d',datdba))) AS acl "- " EXCEPT SELECT unnest(acldefault('d',datdba))) as datacls)"+ "(SELECT array_agg(acl ORDER BY row_n) FROM "+ " (SELECT acl, row_n FROM "+ " unnest(coalesce(datacl,acldefault('d',datdba))) "+ " WITH ORDINALITY AS perm(acl,row_n) "+ " WHERE NOT EXISTS ( "+ " SELECT 1 "+ " FROM unnest(acldefault('d',datdba)) "+ " AS init(init_acl) "+ " WHERE acl = init_acl)) as datacls) "
" AS datacl, "
- "(SELECT array_agg(acl ORDER BY acl::text COLLATE \"C\") FROM ( "- " SELECT unnest(acldefault('d',datdba)) AS acl "- " EXCEPT SELECT unnest(coalesce(datacl,acldefault('d',datdba)))) as rdatacls)"+ "(SELECT array_agg(acl ORDER BY row_n) FROM "+ " (SELECT acl, row_n FROM "+ " unnest(acldefault('d',datdba)) "+ " WITH ORDINALITY AS initp(acl,row_n) "+ " WHERE NOT EXISTS ( "+ " SELECT 1 "+ " FROM unnest(coalesce(datacl,acldefault('d',datdba))) "+ " AS permp(orig_acl) "+ " WHERE acl = orig_acl)) AS rdatacls)"
" AS rdatacl, "
"datistemplate, datconnlimit, "
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = dattablespace) AS tablespace, "
[application/pgp-signature] signature.asc (832B, ../20190520082705.GA1921@paquier.xyz/3-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, bossartn@amazon.com, kommi.haribabu@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15788: 'pg_dump --create' orders database GRANTs incorrectly
In-Reply-To: <20190520082705.GA1921@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox