Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dPDNm-0001DX-GQ for pgsql-bugs@arkaria.postgresql.org; Sun, 25 Jun 2017 19:39:42 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1dPDNl-0007A7-Cj for pgsql-bugs@arkaria.postgresql.org; Sun, 25 Jun 2017 19:39:41 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1dPDN2-0005u9-Pz for pgsql-bugs@postgresql.org; Sun, 25 Jun 2017 19:38:57 +0000 Received: from tamriel.snowman.net ([2001:470:e38f::11]) by makus.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dPDMz-000714-JG for pgsql-bugs@postgresql.org; Sun, 25 Jun 2017 19:38:55 +0000 Received: by tamriel.snowman.net (Postfix, from userid 1000) id D02295F795; Sun, 25 Jun 2017 15:38:52 -0400 (EDT) Date: Sun, 25 Jun 2017 15:38:52 -0400 From: Stephen Frost To: Michael Paquier Cc: Tom Lane , tureba@gmail.com, PostgreSQL mailing lists Subject: Re: BUG #14650: pg_dump -c fails when 'public' schema doesn't exist Message-ID: <20170625193852.GH1769@tamriel.snowman.net> References: <20170512181801.1795.47483@wrigleys.postgresql.org> <4692.1494613340@sss.pgh.pa.us> <20170514154025.GM3151@tamriel.snowman.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="cQx5fYD6zrLPRXWY" Content-Disposition: inline In-Reply-To: <20170514154025.GM3151@tamriel.snowman.net> User-Agent: Mutt/1.5.21 (2010-09-15) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgsql-bugs Precedence: bulk Sender: pgsql-bugs-owner@postgresql.org --cQx5fYD6zrLPRXWY Content-Type: multipart/mixed; boundary="ewiTub5oIxhQg78Q" Content-Disposition: inline --ewiTub5oIxhQg78Q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --ewiTub5oIxhQg78Q Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="fix_pg_dump_public_schema-master_v2.patch" Content-Transfer-Encoding: quoted-printable =46rom 29a5cefadf0213525b57c40c8022654542668829 Mon Sep 17 00:00:00 2001 =46rom: 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 =3D 'public'),0)"); =20 appendPQExpBuffer(query, ") "); =20 diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_du= mp.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 =3D ( 'pg_dump', '--no-sync', '-f', "$tempdir/defaults.sql", 'postgres', ], }, + defaults_no_public =3D> { + database =3D> 'regress_pg_dump_test', + dump_cmd =3D> [ + 'pg_dump', + '--no-sync', + '-f', + "$tempdir/defaults_no_public.sql", + 'regress_pg_dump_test', ], }, + defaults_no_public_clean =3D> { + database =3D> 'regress_pg_dump_test', + dump_cmd =3D> [ + 'pg_dump', + '--no-sync', + '-c', + '-f', + "$tempdir/defaults_no_public_clean.sql", + 'regress_pg_dump_test', ], }, =20 # Do not use --no-sync to give test coverage for data sync. defaults_custom_format =3D> { @@ -4524,6 +4541,35 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM = SQL WITH FUNCTION pg_catalog pg_dumpall_globals_clean =3D> 1, test_schema_plus_blobs =3D> 1, }, }, =20 + 'CREATE SCHEMA public' =3D> { + all_runs =3D> 1, + catch_all =3D> 'CREATE ... commands', + regexp =3D> qr/^CREATE SCHEMA public;/m, + like =3D> { + clean =3D> 1, + clean_if_exists =3D> 1, }, + unlike =3D> { + binary_upgrade =3D> 1, + createdb =3D> 1, + defaults =3D> 1, + exclude_test_table =3D> 1, + exclude_test_table_data =3D> 1, + no_blobs =3D> 1, + no_privs =3D> 1, + no_owner =3D> 1, + only_dump_test_schema =3D> 1, + pg_dumpall_dbprivs =3D> 1, + schema_only =3D> 1, + section_pre_data =3D> 1, + test_schema_plus_blobs =3D> 1, + with_oids =3D> 1, + exclude_dump_test_schema =3D> 1, + only_dump_test_table =3D> 1, + pg_dumpall_globals =3D> 1, + pg_dumpall_globals_clean =3D> 1, + role =3D> 1, + section_post_data =3D> 1, }, }, + 'CREATE SCHEMA dump_test' =3D> { all_runs =3D> 1, catch_all =3D> 'CREATE ... commands', @@ -5219,6 +5265,34 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM = SQL WITH FUNCTION pg_catalog data_only =3D> 1, section_data =3D> 1, }, }, =20 + 'DROP SCHEMA public (for testing without public schema)' =3D> { + all_runs =3D> 1, + database =3D> 'regress_pg_dump_test', + create_order =3D> 100, + create_sql =3D> 'DROP SCHEMA public;', + regexp =3D> qr/^DROP SCHEMA public;/m, + like =3D> { }, + unlike =3D> { defaults_no_public =3D> 1, + defaults_no_public_clean =3D> 1, } }, + + 'DROP SCHEMA public' =3D> { + all_runs =3D> 1, + catch_all =3D> 'DROP ... commands', + regexp =3D> qr/^DROP SCHEMA public;/m, + like =3D> { clean =3D> 1 }, + unlike =3D> { + clean_if_exists =3D> 1, + pg_dumpall_globals_clean =3D> 1, }, }, + + 'DROP SCHEMA IF EXISTS public' =3D> { + all_runs =3D> 1, + catch_all =3D> 'DROP ... commands', + regexp =3D> qr/^DROP SCHEMA IF EXISTS public;/m, + like =3D> { clean_if_exists =3D> 1 }, + unlike =3D> { + clean =3D> 1, + pg_dumpall_globals_clean =3D> 1, }, }, + 'DROP EXTENSION plpgsql' =3D> { all_runs =3D> 1, catch_all =3D> 'DROP ... commands', @@ -6433,6 +6507,9 @@ if ($collation_check_stderr !~ /ERROR: /) $collation_support =3D 1; } =20 +# 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 =3D 12; @@ -6440,6 +6517,11 @@ my $num_tests =3D 12; foreach my $run (sort keys %pgdump_runs) { my $test_key =3D $run; + my $run_db =3D 'postgres'; + + if (defined($pgdump_runs{$run}->{database})) { + $run_db =3D $pgdump_runs{$run}->{database}; + } =20 # 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 =3D 'postgres'; + + # Specific tests can override the database to use + if (defined($tests{$test}->{database})) { + $test_db =3D $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; + } =20 # Skip any collation-related commands if there is no collation support if (!$collation_support && defined($tests{$test}->{collation})) @@ -6507,7 +6602,7 @@ plan tests =3D> $num_tests; # Set up schemas, tables, etc, to be dumped. =20 # Build up the create statements -my $create_sql =3D ''; +my %create_sql =3D (); =20 foreach my $test ( sort { @@ -6529,6 +6624,12 @@ foreach my $test ( } } keys %tests) { + my $test_db =3D 'postgres'; + + if (defined($tests{$test}->{database})) { + $test_db =3D $tests{$test}->{database}; + } + if ($tests{$test}->{create_sql}) { =20 @@ -6539,12 +6640,15 @@ foreach my $test ( } =20 # Add terminating semicolon - $create_sql .=3D $tests{$test}->{create_sql} . ";"; + $create_sql{$test_db} .=3D $tests{$test}->{create_sql} . ";"; } } =20 # 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}); +} =20 ######################################### # Test connecting to a non-existent database --=20 2.7.4 --ewiTub5oIxhQg78Q Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="fix_pg_dump_public_schema-96_v2.patch" Content-Transfer-Encoding: quoted-printable =46rom abc4cb77d428bbb9227e39010e2596bc68242c6f Mon Sep 17 00:00:00 2001 =46rom: 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 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 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 =3D 'public'),0)"); =20 appendPQExpBuffer(query,") "); =20 --=20 2.7.4 --ewiTub5oIxhQg78Q-- --cQx5fYD6zrLPRXWY Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJZUBFMAAoJEO1sijiDR2RVQA8P/2rO9BoQZAZi1dF6o+lX2zvA Su9qs6BPf+qQRxELpNwvWUt53jTOeSxxeaILP4lgZWC6lnkOFjzIoFaYD+6CrSDW NyqsQ5ls055SpLwqGhDYuUv8/3J0Pf0CGiX0EhbIqqwULvtxoyk2eWkSmHLTM3W6 7AS3EP2WKyg8uFiuSNAvThZqldXT0U030KtL7m2pzeb20Z+H1xZpD/azXlzNWe6v uA4kjQZbItdoIJGZW08ALO8nIAvKY+H9ohv2r2InZ/q9zGwZcFHi+DdovqnwXFjn yAPbdF28LcvT8Ep0eqa3JNeB2GqPtx1mvLGUqNl6UoFUAZfUNW0/5xgrmljzmDCY nBeRTI1yssmh5Zc4x45uPLc0M1YhJoNVc7WdI5Q5pzjjwRA4AuTPd6nlAnEIHzN/ 0UKdYUa06z5VRYlwn9Fzqrk+wEEyPgeaKB5gi24PEUeTlIkVTEJARrgKHk8mc5hw cYqtb1XiCC+s91DhHlPayYzvSw0nZz3vVox/uACR+14odW1bDkRXyleM+Bn66a2u tnSY0LFxICmePhx11MLqWgQ9gJP+TyVxR3N20r63pZExQ5QELE2jZNPGZHdezl2l Deurx8qmnKG1zCzbxJDAbJ1gOsNgOs+BJBfmk1Z4Wd7mmvZiMEnG+Lssk77A+0OI OW7Bvl0/HOrSUfL3j0QK =/jZh -----END PGP SIGNATURE----- --cQx5fYD6zrLPRXWY--