From 29a5cefadf0213525b57c40c8022654542668829 Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Sun, 25 Jun 2017 11:27:49 -0400 Subject: [PATCH] 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 --- src/bin/pg_dump/pg_dump.c | 7 ++- src/bin/pg_dump/t/002_pg_dump.pl | 110 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index ec349d4..502d5eb 100644 --- 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 e70a421..a8000db 100644 --- 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 -- 2.7.4