Date: Wed, 22 May 2019 15:26:26 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Bossart, Nathan" <bossartn@amazon.com>
Cc: Haribabu Kommi <kommi.haribabu@gmail.com>,
	"pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #15788: 'pg_dump --create' orders database GRANTs incorrectly
Message-ID: <20190522062626.GC1486@paquier.xyz>
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>
 <20190520082705.GA1921@paquier.xyz>
 <92E61F31-BD06-49B1-950F-8863239E342F@amazon.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="bAmEntskrkuBymla"
Content-Disposition: inline
In-Reply-To: <92E61F31-BD06-49B1-950F-8863239E342F@amazon.com>
Precedence: bulk


--bAmEntskrkuBymla
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, May 20, 2019 at 10:37:50PM +0000, Bossart, Nathan wrote:
> On 5/20/19, 1:27 AM, "Michael Paquier" <michael@paquier.xyz> wrote:
>> 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).
>=20
> Thanks.  Your version looks good to me.

Okay, I have done more tests, checks and tweaks on this stuff, and
pushed the fix for databases down to 9.6.

Now, I have noticed two separate issues while testing:=20
1) We need a similar fix for tablespaces and pg_dumpall
1-1) Take for example this set of commands, reusing your previous
example:
\! rm -rf /tmp/tbspc/
\! mkdir -p /tmp/tbspc/
CREATE ROLE a_user;
CREATE ROLE b_user WITH SUPERUSER;
CREATE ROLE c_user;
CREATE TABLESPACE poo LOCATION '/tmp/tbspc/';
SET SESSION AUTHORIZATION b_user;
REVOKE ALL ON TABLESPACE poo FROM public;
GRANT CREATE ON TABLESPACE poo TO c_user WITH GRANT OPTION;
SET SESSION AUTHORIZATION c_user;
GRANT CREATE ON TABLESPACE poo TO a_user;
1-2) And then run pg_dumpall -g (-t is fine but including roles ensures
a correct test), where you will notice the following GRANT ordering:
CREATE TABLESPACE poo OWNER ioltas LOCATION '/tmp/tbspc';
SET SESSION AUTHORIZATION c_user;
GRANT ALL ON TABLESPACE poo TO a_user; -- breaks here
RESET SESSION AUTHORIZATION;
GRANT ALL ON TABLESPACE poo TO c_user WITH GRANT OPTION;

The problem comes from dumpTablespaces() which still has a broken ACL
ordering logic.  I'll start a new thread about that as this thread is
actually now fixed.  It would be interesting to refactor all that
stuff, but I'd like to think that it would be much more interesting if
we could get pg_init_privs to work with databases and tablespace
instead, but that would not be back-patchable.  So I'll focus on the
bug fix.

2) On HEAD, before the patch, I am able to trigger the following
failure in libpq when using for example pg_dumpall and a 9.5 backend:
pg_dump: column number -1 is out of range 0..36
I am not sure yet what's exactly causing that, but this is just a
heads-up.
--
Michael

--bAmEntskrkuBymla
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAlzk65IACgkQnvQgOdby
QH0n+xAAqHQCk0U8Xtio6Ik25W4bstd/D6h2hae6j5VypSP7pDsiRUX5j/v461bD
rjo46lBA7Zg3WmtFMn8O5UUTtfYdgLF0uXFd3+SrlcFEQcoG/fiyEM79NH3aujKJ
/uRWEFiIhJV2A/GxwzYSXnzvVQ1XwmX0G0rjWH9+SiBr/8WiWtE8bxSIKKTFK38Q
JMvgjl2KgIuvv2Zm11eao2pNdTzfYRtNbn2VjPugduqcZg0n3e/MI7laojN1ZmJ1
PYa5EZuEGieOAGdrBkBgz7oLzO0XfiiSxScE78zFCpsnfsXm2dQFZlOqWuxTy8e0
jBZqn2xJQLEGQJ2fM5199qFARtEv1pUAFWH/1MCVpJvTW8MRFsmNpajIKo+rJmDO
dEPJ7Xpg7JH16gUTzx2Y7VPuRaN7SkqHw8EjivIEa2eHnERdtkhhdboqrcdOGq4N
gfmPrJg+h1ZjRJo/V1B1gFPrJ7gxsirmmPrBjbsAjg4ZkwWIUGFyjlTOspoe81yY
g+ED6P4oaw8yzAArUO+yABN7SgIO5GFjJgtlJbYvMbXcCP/EzXzqoSC+jI6OBa5n
Bhq6dIAWCrAu/vM/CHrc/GcHlWzV+bi17xfpof2sAQJ4hAr4ZncpXtizaVEPhUmu
cEXl/1PrklGrgba+NxHbGfFV0eRGEG1aHcYJC8j5JnLv5wSoRYc=
=CJQ7
-----END PGP SIGNATURE-----

--bAmEntskrkuBymla--