From: Stephen Frost <sfrost@snowman.net>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: tureba@gmail.com, PostgreSQL mailing lists <pgsql-bugs@postgresql.org>
Subject: Re: BUG #14650: pg_dump -c fails when 'public' schema doesn't exist
Date: Sun, 25 Jun 2017 15:38:52 -0400
Message-ID: <20170625193852.GH1769@tamriel.snowman.net> (raw)
In-Reply-To: <20170514154025.GM3151@tamriel.snowman.net>
References: <20170512181801.1795.47483@wrigleys.postgresql.org>
<4692.1494613340@sss.pgh.pa.us>
<CAB7nPqTYuKYS4R=GBqL7tCpN67DPDTNGuf6JcH-RH1_vpEEoLQ@mail.gmail.com>
<20170514154025.GM3151@tamriel.snowman.net>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-bugs>
Micahel, all,
* Stephen Frost (sfrost@snowman.net) wrote:
> I would think using a coalesce() would be a bit cleaner, as in:
[...]
Attached are updated patches for both master and 9.6.
Barring objections, I'll push these tomorrow.
Thanks!
Stephen
Attachments:
[text/x-diff] fix_pg_dump_public_schema-master_v2.patch (7.2K, ../20170625193852.GH1769@tamriel.snowman.net/2-fix_pg_dump_public_schema-master_v2.patch)
download | inline diff:
From 29a5cefadf0213525b57c40c8022654542668829 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
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.cindex 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.plindex 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
[text/x-diff] fix_pg_dump_public_schema-96_v2.patch (1.8K, ../20170625193852.GH1769@tamriel.snowman.net/3-fix_pg_dump_public_schema-96_v2.patch)
download | inline diff:
From abc4cb77d428bbb9227e39010e2596bc68242c6f Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
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 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.cindex 27a2a0c..d1db370 100644--- a/src/bin/pg_dump/pg_dump.c+++ b/src/bin/pg_dump/pg_dump.c@@ -3659,9 +3659,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,") ");
--
2.7.4
[application/pgp-signature] signature.asc (818B, ../20170625193852.GH1769@tamriel.snowman.net/4-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: sfrost@snowman.net, michael.paquier@gmail.com, tgl@sss.pgh.pa.us
Subject: Re: BUG #14650: pg_dump -c fails when 'public' schema doesn't exist
In-Reply-To: <20170625193852.GH1769@tamriel.snowman.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox