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