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

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> 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.

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).  With this, I get empty dumps
for the default state of template1 and postgres, which is what one
really would expect.  And testing shows that if you modify the ACL
of language plpgsql, that does still come through as expected.

(Note: this breaks the parts of the pg_dump regression tests that
expect to see "CREATE LANGUAGE plpgsql".  I've not bothered to update
those, pending the decision on whether to do this.)

            regards, tom lane

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d65ea54..87b15a1 100644
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** selectDumpableAccessMethod(AccessMethodI
*** 1617,1637 ****
   * selectDumpableExtension: policy-setting subroutine
   *        Mark an extension as to be dumped or not
   *
!  * Normally, we dump all extensions, or none of them if include_everything
!  * is false (i.e., a --schema or --table switch was given).  However, in
!  * binary-upgrade mode it's necessary to skip built-in extensions, since we
   * assume those will already be installed in the target database.  We identify
   * such extensions by their having OIDs in the range reserved for initdb.
   */
  static void
  selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
  {
      /*
!      * Use DUMP_COMPONENT_ACL for from-initdb extensions, to allow users to
!      * change permissions on those objects, if they wish to, and have those
!      * changes preserved.
       */
!     if (dopt->binary_upgrade && extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
          extinfo->dobj.dump = extinfo->dobj.dump_contains = DUMP_COMPONENT_ACL;
      else
          extinfo->dobj.dump = extinfo->dobj.dump_contains =
--- 1617,1637 ----
   * selectDumpableExtension: policy-setting subroutine
   *        Mark an extension as to be dumped or not
   *
!  * Built-in extensions should be skipped except for checking ACLs, since we
   * assume those will already be installed in the target database.  We identify
   * such extensions by their having OIDs in the range reserved for initdb.
+  * We dump all user-added extensions by default, or none of them if
+  * include_everything is false (i.e., a --schema or --table switch was given).
   */
  static void
  selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
  {
      /*
!      * Use DUMP_COMPONENT_ACL for built-in extensions, to allow users to
!      * change permissions on their member objects, if they wish to, and have
!      * those changes preserved.
       */
!     if (extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
          extinfo->dobj.dump = extinfo->dobj.dump_contains = DUMP_COMPONENT_ACL;
      else
          extinfo->dobj.dump = extinfo->dobj.dump_contains =