Re: BUG #15788: 'pg_dump --create' orders database GRANTs incorrectly

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).
>
> 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:
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