Re: [GENERAL] missing public on schema public - Mailing list pgsql-general

Tom, all,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Bo Thorbjørn Jensen <bo@budget123.dk> writes:
> > I have some additional info and a fix.
> > Firstly steps to reproduce:
>
> Yeah, I can reproduce this.  I suspect it got broken by Stephen's hacking
> around with default ACLs.  A simple example is

Yes, it's related to the work I did with pg_dump's ACL handling, because
we're no longer just always including the whole revoke/grant set of ACLs
for everything in the output.

> $ pg_dump -c -U postgres postgres | grep -i public
> DROP SCHEMA public;
> -- Name: public; Type: SCHEMA; Schema: -; Owner: postgres
> CREATE SCHEMA public;
> ALTER SCHEMA public OWNER TO postgres;
> -- Name: SCHEMA public; Type: COMMENT; Schema: -; Owner: postgres
> COMMENT ON SCHEMA public IS 'standard public schema';
> -- Name: public; Type: ACL; Schema: -; Owner: postgres
> GRANT ALL ON SCHEMA public TO PUBLIC;
>
> That's fine, but if I shove it through an archive file:

This works because I added into pg_dump.c a check based on if the output
is clean (and therefore the public schema is being recreated or not).

In hindsight, that wasn't really the right thing to do because it ends
up only working when pg_dump is run with -c and doesn't consider the
case where pg_dump is run without -c but pg_restore is.

> $ pg_dump -f p.dump -Fc -U postgres postgres
>
> $ pg_restore -c p.dump | grep -i public

This doesn't work because pg_dump isn't run with -c, while pg_restore
is.  If the archive is created with pg_dump -c (as the above was), then
the results match up between the two runs.  Note also that if pg_dump is
run with -c then a pg_restore without -c would actually still include
the GRANT statement, which isn't really correct either.

That's obviously a change from what we had before and wasn't
intentional.

> This is *REALLY BAD*.  Quite aside from the restore being wrong,
> those two sequences should never ever give different results.
> Stephen, you put some filtering logic in the wrong place in pg_dump.

I do wish it was that simple.

Unfortunately, the public schema is just ridiculously special, both in
the way it's a 'user' object but is created by initdb and that it's got
special non-default ACLs on it and how it has explicit special code to
skip over it when a restore is happening, unless -c is used.

What I'm afraid we need to do here is basically continue to hack on that
code in pg_backup_archiver.c's _printTocEntry() to teach it to issue the
default GRANT ALL ON SCHEMA public TO PUBLIC; when we are processing the
TOC entry for CREATE SCHEMA public;.

That would make the recreation of the public schema when pg_dump or
pg_restore is being run with -c actually match how the public schema is
created by initdb, and the rest would end up falling into place, I
think.

One complication, however, is what happens when a user drops and
recreates the public schema.  If that's done, we'll end up not dumping
out the delta from the public schema's initial ACLs, which wouldn't be
correct if you're restoring into a newly initdb'd cluster.  I'm thinking
that we need to forcibly look at the delta from
public-as-installed-by-initdb and whatever-public-is-now, regardless of
if the public schema was recreated by the user or not, because on
restore we are expecting a newly initdb'd cluster with the public schema
as originally installed (or as installed by pg_dump/pg_restore following
the logic above).

I'll play around with this approach and see if things end up working out
in a better fashion with it.  Baking this knowledge into
pg_backup_archiver.c is certainly ugly, but handling of public has
always been hard-coded into that, and we even added more special
handling to that code 10 years ago to deal with the COMMENT on the
public schema, so this is really just more of the same.

Thanks!

Stephen