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

Tom,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> > On Monday, January 22, 2018, Stephen Frost <sfrost@snowman.net> wrote:
> >> In the end, I feel like this patch has actually been through the ringer
> >> and back because it was brought up in the context of solving a problem
> >> that we'd all like to fix in a better way.  Had it been simply dropped
> >> on us as a "I'd like to not have comments in my pg_dump output and
> >> here's a patch to allow me to do that" I suspect it would have been
> >> committed without anywhere near this level of effort.
>
> Indeed ... but it was *not* presented that way, and that's what makes
> this somewhat of a difficult call.  You and Robert argued that there were
> valid use-cases, but I feel like that was somewhat of an after-the-fact
> justification, rather than an actual organic feature request.

This was definitely the kind of follow-on work that I was anticipating
happening with the rework that was done to dump_contains in a9f0e8e5a2e.
I would think that we'd support enable/disable of all of the different
component types that pg_dump recognizes, and we're most of the way there
at this point.  The comment above the #define's in pg_dump.h even hints
at that- component types of an object which can be selected for dumping.

> "David G. Johnston" <david.g.johnston@gmail.com> writes:
> > +0; but recognizing our users are going to remain annoyed by the existing
> > problem and that telling them that this is the answer will not sit well.
>
> FWIW, today's pg_dump refactoring got rid of one of the use-cases for
> this, namely the COMMENT ON DATABASE aspect.  We got rid of another aspect
> (creating/commenting on the public schema) some time ago, via a method
> that was admittedly a bit of a hack but got the job done.  What seems to
> be left is that given a default installation, pg_dump will emit a
> "COMMENT ON EXTENSION plpgsql" that can't be restored by an unprivileged
> user ... as well as a "CREATE EXTENSION IF NOT EXISTS plpgsql", which is
> an utter kluge.  Maybe we can think of a rule that will exclude plpgsql
> from dumps without causing too much havoc.

Having a generic --exclude-extension=plpgsql might be interesting..  I
can certainly recall wishing for such an option when working with
postgis.

> The most obvious hack is just to exclude PL objects with OIDs below 16384
> from the dump.  An issue with that is that it'd lose any nondefault
> changes to the ACL for plpgsql, which seems like a problem.  On the
> other hand, I think the rule we have in place for the public schema is
> preventing dumping local adjustments to that, and there have been no
> complaints.  (Maybe I'm wrong and the initacl mechanism handles that
> case?  If so, seems like we could get it to handle local changes to
> plpgsql's ACL as well.)

Both the public schema and plpgsql's ACLs should be handled properly
(with local changes preserved) through the pg_init_privs system.  I'm
not sure what you're referring to regarding a rule preventing dumping
local adjustments to the public schema, as far as I can recall we've
basically always supported that.

Losing non-default ACLs for plpgsql seems like a step backwards.

Frankly, the comment on plpgsql is probably one of the most worthless
comments we have anyway- all it does is re-state information you
already know: it's a procedural language called 'plpgsql'.  I'd
suggest we could probably survive without it, though that is a long
route to "fixing" this, though at least we could tell people it's been
fixed in newer versions and there's a kludge available until then..

Thanks!

Stephen