Re: Extensions not dumped when --schema is used

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.