On Thu, Feb 18, 2021 at 11:13:06AM +0100, Guillaume Lelarge wrote:
> I finally managed to get a working TAP test for my patch. I have no idea if
> it's good, and if it's enough. Anyway, new version of the patch attached.

As presented in this patch, specifying both --extension and
--table/--schema means that pg_dump will dump both tables and
extensions matching the pattern passed down.  But shouldn't extensions
not be dumped if --table or --schema is used?  Combining --schema with
--table implies that the schema part is ignored, for instance.

The comment at the top of selectDumpableExtension() is incorrect,
missing the new case added by --extension.

The use of strict_names looks right to me, but you have missed a
refresh of the documentation of --strict-names that applies to
--extension with this patch.

+       dump_cmd => [
+           'pg_dump', "--file=$tempdir/test_schema_plus_extensions.sql",
+           '--schema=dump_test', '--extension=plpgsql', '--no-sync',
What's the goal of this test.  plpgsql is a system extension so it
will never be dumped.  It seems to me that any tests related to this 
patch should be added to src/test/modules/test_pg_dump/, which
includes a custom extension in the test, that could be dumped.

+        dumped.  Multiple schemas can be selected by writing multiple
+        <option>-e</option> switches.  The <replaceable
s/schemas/extensions/.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to