From: Bossart, Nathan <bossartn@amazon.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Neil Chen <carpenter.nail.cz@gmail.com>
Cc: Boris P. Korzun <drtr0jan@yandex.ru>
Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: Inconsistent behavior of pg_dump/pg_restore on DEFAULT PRIVILEGES
Date: Tue, 19 Oct 2021 03:54:37 +0000
Message-ID: <FD480C32-0B10-4054-B2CB-6DEEA7B0B0E0@amazon.com> (raw)
In-Reply-To: <1737876.1634613552@sss.pgh.pa.us>
References: <CAA3qoJnr2+1dVJObNtfec=qW4Z0nz=A9+r5bZKoTSy5RDjskMw@mail.gmail.com>
<8261632200581@mail.yandex.ru>
<CAA3qoJ=f6jZAkkfwH+rtH_8C9z-nfcgNFYZnGYxVdBjz7Vkhig@mail.gmail.com>
<CAD21AoBrCbjiJt3OCDQCLrM6OQOxjV9cAyM6bn4=SjHq0konBQ@mail.gmail.com>
<CAD21AoC+m46JEsFE2d6JQuPuNN5ODj1vpZ3z2HtPdn+HFDJfdg@mail.gmail.com>
<721942.1634180151@sss.pgh.pa.us>
<CAD21AoAUCNCo77LJLAFGEkhtgruMpzPD153SC7dY7TyahvM_Fg@mail.gmail.com>
<YWfh9O/36r+GlgNB@paquier.xyz>
<CAD21AoDRg1E7AEM_5ifyjFbQ71J303OHA-R1HjzqC0FT306dKQ@mail.gmail.com>
<AB89F451-ED7F-4FA6-ADF8-F8132220D6A2@amazon.com>
<CAD21AoArNPsNKYE1b-oURYonUA7Zo-kaq-u661uWcZqTURb48A@mail.gmail.com>
<1737876.1634613552@sss.pgh.pa.us>
On 10/18/21, 8:20 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> Masahiko Sawada <sawada.mshk@gmail.com> writes:
>> I've looked at the patch proposed you proposed. If we can depend on
>> acldefault() being STRICT (which is legitimate to me), I think we
>> don't need to build an expression depending on the caller (i.g.,
>> is_default_acl). If acldefault() were to become not STRICT, we could
>> detect it by regression tests. What do you think?
>
> FWIW, I'm working on a refactoring of this logic that will bring the
> acldefault() call into the getDefaultACLs code, which would mean that
> we won't need that assumption anymore anyway. The code as I have it
> produces SQL like
Nice!
> So I think we don't need to worry about whether acldefault() will stay
> strict. This patch will only need to work in the back branches.
This seems fine to me, too. I don't think relying on STRICT is any
better or worse than adding a flag for this one special case.
+ /*
+ * Since the default for a global entry is the hard-wired default
+ * ACL for the particular object type, we pass defaclobjtype except
+ * for the case of 'S' (DEFACLOBJ_SEQUENCE) where we need to
+ * transform it to 's' since acldefault() SQL-callable function
+ * handles 's' as a sequence. On the other hand, since the default
+ * for non-global entries is an empty ACL we pass NULL. This works
+ * because acldefault() is STRICT.
+ */
I'd split out the two special cases in the comment. What do you think
about something like the following?
/*
* Build the expression for determining the object type.
*
* While pg_default_acl uses 'S' for sequences, acldefault()
* uses 's', so we must transform 'S' to 's'.
*
* The default for a schema-local default ACL (i.e., entries
* in pg_default_acl with defaclnamespace != 0) is an empty
* ACL. We use NULL as the object type for those entries,
* which forces acldefault() to also return NULL because it is
* STRICT.
*/
+ create_sql => 'ALTER DEFAULT PRIVILEGES
+ FOR ROLE regress_dump_test_role IN SCHEMA dump_test
+ GRANT EXECUTE ON FUNCTIONS TO regress_dump_test_role;',
+ regexp => qr/^
+ \QALTER DEFAULT PRIVILEGES \E
+ \QFOR ROLE regress_dump_test_role IN SCHEMA dump_test \E
+ \QGRANT ALL ON FUNCTIONS TO regress_dump_test_role;\E
+ /xm,
It could be a bit confusing that create_sql uses "GRANT EXECUTE" but
we expect to see "GRANT ALL." IIUC this is correct, but maybe we
should use "GRANT ALL" in create_sql so that they match.
Nathan
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: bossartn@amazon.com, tgl@sss.pgh.pa.us, sawada.mshk@gmail.com, michael@paquier.xyz, carpenter.nail.cz@gmail.com, drtr0jan@yandex.ru, pgsql-bugs@lists.postgresql.org
Subject: Re: Inconsistent behavior of pg_dump/pg_restore on DEFAULT PRIVILEGES
In-Reply-To: <FD480C32-0B10-4054-B2CB-6DEEA7B0B0E0@amazon.com>
* 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