Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump

Tom,

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> I wrote:
> > I went looking and realized that actually what we're interested in here
> > is the plpgsql extension, not the plpgsql language ... and in fact the
> > behavior I was thinking of is already there, except for some reason it's
> > only applied during binary upgrade. So I think we should give serious
> > consideration to the attached, which just removes the binary_upgrade
> > condition (and adjusts nearby comments).
>
> In further testing of that, I noticed that it made the behavior of our
> other bugaboo, the public schema, rather inconsistent. With this
> builtin-extensions hack, the plpgsql extension doesn't get dumped,
> whether or not you say "clean". But the public schema does get
> dropped and recreated if you say "clean". That's not helpful for
> non-superuser users of pg_dump, so I think we should try to fix it.

I'm not entirely sure about trying to also support --clean for
non-superusers.. We've long had that the public schema is dropped and
recreated with --clean and it seems likely that at least some users are
depending on us doing that. In any case, it's certainly not a change
that I think we could backpatch. Perhaps we could just change it moving
forward (which would make me happier, really, since what I think we do
with these initdb-time things currently is a bit bizarre).

> Hence, the attached updated patch also makes selection of the public
> schema use the DUMP_COMPONENT infrastructure rather than hardwired
> code.
>
> I note btw that the removed bit in getNamespaces() is flat out wrong
> independently of these other considerations, because it makes the SQL
> put into an archive file vary depending on whether --clean was specified
> at pg_dump time. That's not supposed to happen.

Yes, having that in getNamespaces() isn't correct but we need to do
something there, and I've been trying to figure out what. The reason
it's there in the first place is that if you do --clean then the public
schema is dropped and recreated *but* the initdb-time ACL isn't put back
for it (and there's no ACL entries in the dump file, be it text or
custom, if the user didn't change the ACL from the initdb-time one).

What I was planning to do there was somehow inject the initdb-time ACL
for public into the set of things to restore when we're asked to do a
restore from a custom-format (or directory or other type which is
handled by pg_restore) dump. We have to account for someone asking for
pg_dump --clean --no-privileges also though. We would still need to do
something for text-based output though..

Note that this fix really needs to be back-patched and that we had
complaints about the --clean issue with the public schema on text-based
pg_dump's (which is why the hack was added to getNamespaces() in the
first place) and, more recently, for non-text based pg_dump's.

Thanks!

Stephen