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, 14 May 2017 11:40:25 -0400
Message-ID: <20170514154025.GM3151@tamriel.snowman.net> (raw)
In-Reply-To: <CAB7nPqTYuKYS4R=GBqL7tCpN67DPDTNGuf6JcH-RH1_vpEEoLQ@mail.gmail.com>
References: <20170512181801.1795.47483@wrigleys.postgresql.org>
<4692.1494613340@sss.pgh.pa.us>
<CAB7nPqTYuKYS4R=GBqL7tCpN67DPDTNGuf6JcH-RH1_vpEEoLQ@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-bugs>
* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Sat, May 13, 2017 at 3:22 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > tureba@gmail.com writes:
> >> It seems that pg_dump -c fails when the database doesn't have a 'public'
> >> schema. When using a clean database, then dropping (or renaming) the
> >> 'public' schema, a subsequent pg_dump -c tells me:
> >
> >> $ pg_dump -c test
> >> pg_dump: [archiver (db)] query failed: ERROR: schema "public" does not exist
> >> LINE 1: ...::regclass AND pip.objsubid = 0 AND pip.objoid <> 'public'::...
> >> ^
> >
> >> This seems to have started in commit 330b84d8c4
> >
> > Yeah, this is not cool. Stephen?
>
> This is visibly an oversight of the recent commit 330b84d, which is
> broken on its face because it does not check for the existence of this
> schema. One simple fix would be to extend this query with a SELECT
> CASE ... ELSE 0 FROM pg_namespace WHERE nspname = 'public' but that's
> ugly.
I would think using a coalesce() would be a bit cleaner, as in:
! appendPQExpBuffer(query,
! " AND pip.objoid <> "
! "coalesce((select oid from pg_namespace "
! "where nspname = 'public'),0)");
Attached is a patch which fixes this issue in that way and includes
changes to the TAP tests to check this case. The TAP tests are a bit
much to back-patch, I guess, since it involves adding the ability to
have multiple databases to the 002_pg_dump.pl framework, but hopefully
there won't be objections to adding that into master. This may also
provide an initial step towards folding 010_dump_connstr.pl into
002_pg_dump.pl and therefore possibly removing the need for another
cluster to be created during the pg_dump TAP tests and hopefully
reducing their overall runtime.
Thanks!
Stephen
Attachments:
[text/x-diff] fix_pg_dump_no_public_schema_v1-master.patch (6.8K, ../20170514154025.GM3151@tamriel.snowman.net/2-fix_pg_dump_no_public_schema_v1-master.patch)
download | inline diff:diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.cnew file mode 100644index b26358c..d331c46
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** getNamespaces(Archive *fout, int *numNam
*** 4130,4138 ****
* 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.
*/
if (dopt->outputClean)
! appendPQExpBuffer(query," AND pip.objoid <> 'public'::regnamespace");
appendPQExpBuffer(query,") ");
--- 4130,4144 ----
* 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 <> "
! "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.plnew file mode 100644index 9bd400e..19b40de
*** a/src/bin/pg_dump/t/002_pg_dump.pl
--- b/src/bin/pg_dump/t/002_pg_dump.pl
*************** my %pgdump_runs = (
*** 103,108 ****
--- 103,125 ----
'-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 => {
test_key => 'defaults',
*************** qr/CREATE TRANSFORM FOR integer LANGUAGE
*** 4513,4518 ****
--- 4530,4572 ----
pg_dumpall_globals_clean => 1,
test_schema_plus_blobs => 1, }, },
+ 'CREATE SCHEMA public (none exists)' => {+ all_runs => 1,+ database => 'regress_pg_dump_test',+ regexp => qr/^DROP SCHEMA public;/m,+ like => { },+ unlike => { defaults_no_public => 1,+ defaults_no_public_clean => 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',
*************** qr/CREATE TRANSFORM FOR integer LANGUAGE
*** 5205,5210 ****
--- 5259,5274 ----
data_only => 1,
section_data => 1, }, },
+ 'DROP SCHEMA public' => {+ 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 EXTENSION plpgsql' => {
all_runs => 1,
catch_all => 'DROP ... commands',
*************** if ($collation_check_stderr !~ /ERROR: /
*** 6411,6416 ****
--- 6475,6483 ----
$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;
*************** my $num_tests = 12;
*** 6418,6423 ****
--- 6485,6495 ----
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++;
*************** foreach my $run (sort keys %pgdump_runs)
*** 6436,6441 ****
--- 6508,6523 ----
# Then count all the tests run against each run
foreach my $test (sort keys %tests)
{
+ my $test_db = 'postgres';+ + if (defined($tests{$test}->{database})) {+ $test_db = $tests{$test}->{database};+ }+ + 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})) {
next;
*************** plan tests => $num_tests;
*** 6483,6489 ****
# Set up schemas, tables, etc, to be dumped.
# Build up the create statements
! my $create_sql = '';
foreach my $test (
sort {
--- 6565,6571 ----
# Set up schemas, tables, etc, to be dumped.
# Build up the create statements
! my %create_sql = ();
foreach my $test (
sort {
*************** foreach my $test (
*** 6505,6510 ****
--- 6587,6598 ----
}
} keys %tests)
{
+ my $test_db = 'postgres';+ + if (defined($tests{$test}->{database})) {+ $test_db = $tests{$test}->{database};+ }+
if ($tests{$test}->{create_sql})
{
# Skip any collation-related commands if there is no collation support
*************** foreach my $test (
*** 6513,6524 ****
}
# Add terminating semicolon
! $create_sql .= $tests{$test}->{create_sql} . ";";
}
}
# Send the combined set of commands to psql
! $node->safe_psql('postgres', $create_sql);
#########################################
# Test connecting to a non-existent database
--- 6601,6615 ----
}
# Add terminating semicolon
! $create_sql{$test_db} .= $tests{$test}->{create_sql} . ";";
}
}
# Send the combined set of commands to psql
! foreach my $db (sort keys %create_sql)
! {
! $node->safe_psql($db, $create_sql{$db});
! }
#########################################
# Test connecting to a non-existent database
*************** command_fails_like(
*** 6565,6571 ****
foreach my $run (sort keys %pgdump_runs)
{
-
my $test_key = $run;
$node->command_ok(\@{ $pgdump_runs{$run}->{dump_cmd} },
--- 6656,6661 ----
[application/pgp-signature] signature.asc (818B, ../20170514154025.GM3151@tamriel.snowman.net/3-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: <20170514154025.GM3151@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