Do not require 'public' to exist for pg_dump -c

Commit 330b84d8c4 didn't contemplate the case where the public schema
has been dropped and introduced a query which fails when there is no
public schema into pg_dump (when used with -c).

Adjust the query used by pg_dump to handle the case where the public
schema doesn't exist and add tests to check that such a case no longer
fails.

Back-patch the specific fix to 9.6, as the prior commit was.

Adding tests for this case involved adding support to the pg_dump
TAP tests to work with multiple databases, which, while not a large
change, is a bit much to back-patch, so that's only done in master.

Addresses bug #14650
Discussion: https://www.postgresql.org/message-id/20170512181801.1795.47483%40wrigleys.postgresql.org

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ec349d4192c9439d40b7afe700a3564737fee0f6..502d5eb7af5d29071d96703b192f9ef8171b3c1e 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4135,9 +4135,14 @@ getNamespaces(Archive *fout, int *numNamespaces)
                 * essentially a no-op because the new public schema won't have an
                 * entry in pg_init_privs anyway, as the entry will be removed when
                 * the public schema is dropped.
+                *
+                * Further, we have to handle the case where the public schema does
+                * not exist at all.
                 */
                if (dopt->outputClean)
-                       appendPQExpBuffer(query, " AND pip.objoid <> 'public'::regnamespace");
+                       appendPQExpBuffer(query, " AND pip.objoid <> "
+                                                                        "coalesce((select oid from pg_namespace "
+                                                                        "where nspname = 'public'),0)");
 
                appendPQExpBuffer(query, ") ");
 

diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index e70a4213759d980df492767689f44d9e294bd2cb..a8000db3369c34672011399c5ab8671b378a7c2d 100644 (file)
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -97,6 +97,23 @@ my %pgdump_runs = (
                        'pg_dump', '--no-sync',
                        '-f',      "$tempdir/defaults.sql",
                        'postgres', ], },
+       defaults_no_public => {
+               database => 'regress_pg_dump_test',
+               dump_cmd => [
+                       'pg_dump',
+                       '--no-sync',
+                       '-f',
+                       "$tempdir/defaults_no_public.sql",
+                       'regress_pg_dump_test', ], },
+       defaults_no_public_clean => {
+               database => 'regress_pg_dump_test',
+               dump_cmd => [
+                       'pg_dump',
+                       '--no-sync',
+                       '-c',
+                       '-f',
+                       "$tempdir/defaults_no_public_clean.sql",
+                       'regress_pg_dump_test', ], },
 
        # Do not use --no-sync to give test coverage for data sync.
        defaults_custom_format => {
@@ -4524,6 +4541,35 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
                        pg_dumpall_globals_clean => 1,
                        test_schema_plus_blobs   => 1, }, },
 
+       'CREATE SCHEMA public' => {
+               all_runs     => 1,
+               catch_all    => 'CREATE ... commands',
+               regexp       => qr/^CREATE SCHEMA public;/m,
+               like         => {
+                       clean                   => 1,
+                       clean_if_exists         => 1, },
+               unlike => {
+                       binary_upgrade           => 1,
+                       createdb                 => 1,
+                       defaults                 => 1,
+                       exclude_test_table       => 1,
+                       exclude_test_table_data  => 1,
+                       no_blobs                 => 1,
+                       no_privs                 => 1,
+                       no_owner                 => 1,
+                       only_dump_test_schema    => 1,
+                       pg_dumpall_dbprivs       => 1,
+                       schema_only              => 1,
+                       section_pre_data         => 1,
+                       test_schema_plus_blobs   => 1,
+                       with_oids                => 1,
+                       exclude_dump_test_schema => 1,
+                       only_dump_test_table     => 1,
+                       pg_dumpall_globals       => 1,
+                       pg_dumpall_globals_clean => 1,
+                       role                     => 1,
+                       section_post_data        => 1, }, },
+
        'CREATE SCHEMA dump_test' => {
                all_runs     => 1,
                catch_all    => 'CREATE ... commands',
@@ -5219,6 +5265,34 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
                        data_only      => 1,
                        section_data   => 1, }, },
 
+       'DROP SCHEMA public (for testing without public schema)' => {
+               all_runs  => 1,
+               database => 'regress_pg_dump_test',
+               create_order => 100,
+               create_sql => 'DROP SCHEMA public;',
+               regexp    => qr/^DROP SCHEMA public;/m,
+               like      => { },
+               unlike    => { defaults_no_public => 1,
+                              defaults_no_public_clean => 1, } },
+
+       'DROP SCHEMA public' => {
+               all_runs  => 1,
+               catch_all => 'DROP ... commands',
+               regexp    => qr/^DROP SCHEMA public;/m,
+               like         => { clean => 1 },
+               unlike => {
+                       clean_if_exists         => 1,
+                       pg_dumpall_globals_clean => 1, }, },
+
+       'DROP SCHEMA IF EXISTS public' => {
+               all_runs  => 1,
+               catch_all => 'DROP ... commands',
+               regexp    => qr/^DROP SCHEMA IF EXISTS public;/m,
+               like         => { clean_if_exists => 1 },
+               unlike => {
+                       clean => 1,
+                       pg_dumpall_globals_clean => 1, }, },
+
        'DROP EXTENSION plpgsql' => {
                all_runs  => 1,
                catch_all => 'DROP ... commands',
@@ -6433,6 +6507,9 @@ if ($collation_check_stderr !~ /ERROR: /)
        $collation_support = 1;
 }
 
+# Create a second database for certain tests to work against
+$node->psql('postgres','create database regress_pg_dump_test;');
+
 # Start with number of command_fails_like()*2 tests below (each
 # command_fails_like is actually 2 tests)
 my $num_tests = 12;
@@ -6440,6 +6517,11 @@ my $num_tests = 12;
 foreach my $run (sort keys %pgdump_runs)
 {
        my $test_key = $run;
+       my $run_db = 'postgres';
+
+       if (defined($pgdump_runs{$run}->{database})) {
+               $run_db = $pgdump_runs{$run}->{database};
+       }
 
        # Each run of pg_dump is a test itself
        $num_tests++;
@@ -6458,6 +6540,19 @@ foreach my $run (sort keys %pgdump_runs)
        # Then count all the tests run against each run
        foreach my $test (sort keys %tests)
        {
+               # postgres is the default database, if it isn't overridden
+               my $test_db = 'postgres';
+
+               # Specific tests can override the database to use
+               if (defined($tests{$test}->{database})) {
+                       $test_db = $tests{$test}->{database};
+               }
+
+               # The database to test against needs to match the database the run is
+               # for, so skip combinations where they don't match up.
+               if ($run_db ne $test_db) {
+                       next;
+               }
 
                # Skip any collation-related commands if there is no collation support
                if (!$collation_support && defined($tests{$test}->{collation}))
@@ -6507,7 +6602,7 @@ plan tests => $num_tests;
 # Set up schemas, tables, etc, to be dumped.
 
 # Build up the create statements
-my $create_sql = '';
+my %create_sql = ();
 
 foreach my $test (
        sort {
@@ -6529,6 +6624,12 @@ foreach my $test (
                }
        } keys %tests)
 {
+       my $test_db = 'postgres';
+
+       if (defined($tests{$test}->{database})) {
+               $test_db = $tests{$test}->{database};
+       }
+
        if ($tests{$test}->{create_sql})
        {
 
@@ -6539,12 +6640,15 @@ foreach my $test (
                }
 
                # Add terminating semicolon
-               $create_sql .= $tests{$test}->{create_sql} . ";";
+               $create_sql{$test_db} .= $tests{$test}->{create_sql} . ";";
        }
 }
 
 # Send the combined set of commands to psql
-$node->safe_psql('postgres', $create_sql);
+foreach my $db (sort keys %create_sql)
+{
+       $node->safe_psql($db, $create_sql{$db});
+}
 
 #########################################
 # Test connecting to a non-existent database