Re: Extensions not dumped when --schema is used - Mailing list pgsql-hackers

On Wed, Apr 14, 2021 at 10:38:17AM +0900, Michael Paquier wrote:
> On Tue, Apr 13, 2021 at 08:00:34AM -0700, Noah Misch wrote:
> > On Tue, Apr 13, 2021 at 02:43:11PM +0900, Michael Paquier wrote:
> >>> - If extschema='public', "pg_dump -e plpgsql --schema=public" includes
> >>>   commands to dump the relation data.  This surprised me.  (The
> >>>   --schema=public argument causes selectDumpableNamespace() to set
> >>>   nsinfo->dobj.dump=DUMP_COMPONENT_ALL instead of DUMP_COMPONENT_ACL.)

> > This isn't a problem of selecting schemas for inclusion in the dump.  This is
> > a problem of associating extensions with their pg_extension_config_dump()
> > relations and omitting those extension-member relations when "-e" causes
> > omission of the extension.
> 
> At code level, the decision to dump the data of any extension's
> dumpable table is done in processExtensionTables().  I have to admit
> that your feeling here keeps the code simpler than what I have been
> thinking if we apply an extra filtering based on the list of
> extensions in this code path.  So I can see the value in your argument
> to not dump at all the data of an extension's dumpable table as long
> as its extension is not listed, and this, even if its schema is
> explicitly listed.
> 
> So I got down to make the behavior more consistent with the patch
> attached.  This passes your case.

Yes.

> It is worth noting that if a
> table is part of a schema created by an extension, but that the table
> is not dependent on the extension, we would still dump its data if
> using --schema with this table's schema while the extension is not
> part of the list from --extension.  In the attached, that's just the
> extra test with without_extension_implicit_schema.

That's consistent with v13, and it seems fine.  I've not used a non-test
extension that creates a schema.

> --- a/src/test/modules/test_pg_dump/t/001_base.pl
> +++ b/src/test/modules/test_pg_dump/t/001_base.pl
> @@ -208,6 +208,30 @@ my %pgdump_runs = (
>              'pg_dump', '--no-sync', "--file=$tempdir/without_extension.sql",
>              '--extension=plpgsql', 'postgres',
>          ],
> +    },
> +
> +    # plgsql in the list of extensions blocks the dump of extension
> +    # test_pg_dump.
> +    without_extension_explicit_schema => {
> +        dump_cmd => [
> +            'pg_dump',
> +            '--no-sync',
> +            "--file=$tempdir/without_extension_explicit_schema.sql",
> +            '--extension=plpgsql',
> +            '--schema=public',
> +            'postgres',
> +        ],
> +    },
> +
> +    without_extension_implicit_schema => {
> +        dump_cmd => [
> +            'pg_dump',
> +            '--no-sync',
> +            "--file=$tempdir/without_extension_implicit_schema.sql",
> +            '--extension=plpgsql',
> +            '--schema=regress_pg_dump_schema',
> +            'postgres',
> +        ],
>      },);

The name "without_extension_explicit_schema" arose because that test differs
from the "without_extension" test by adding --schema=public.  The test named
"without_extension_implicit_schema" differs from "without_extension" by adding
--schema=regress_pg_dump_schema, so the word "implicit" feels not-descriptive
of the test.  I recommend picking a different name.  Other than that, the
change looks good.