Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d9vdK-0008Mh-CW for pgsql-bugs@arkaria.postgresql.org; Sun, 14 May 2017 15:40:34 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d9vdJ-0008C1-CK for pgsql-bugs@arkaria.postgresql.org; Sun, 14 May 2017 15:40:33 +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 1d9vdI-0008Bs-NC for pgsql-bugs@postgresql.org; Sun, 14 May 2017 15:40:32 +0000 Received: from tamriel.snowman.net ([2001:470:e38f::11]) by makus.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d9vdB-0006H3-NO for pgsql-bugs@postgresql.org; Sun, 14 May 2017 15:40:31 +0000 Received: by tamriel.snowman.net (Postfix, from userid 1000) id 27B815F795; Sun, 14 May 2017 11:40:25 -0400 (EDT) Date: Sun, 14 May 2017 11:40:25 -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: <20170514154025.GM3151@tamriel.snowman.net> References: <20170512181801.1795.47483@wrigleys.postgresql.org> <4692.1494613340@sss.pgh.pa.us> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="K90HjsLT/gE7/naG" Content-Disposition: inline In-Reply-To: X-Editor: Vim http://www.vim.org/ X-Info: http://www.snowman.net X-Operating-System: Linux/3.13.0-117-generic (x86_64) X-Uptime: 10:20:26 up 5 days, 14:53, 6 users, load average: 0.14, 0.21, 0.22 User-Agent: Mutt/1.5.21 (2010-09-15) X-Pg-Spam-Score: -1.9 (-) 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 --K90HjsLT/gE7/naG Content-Type: multipart/mixed; boundary="EVx8OOgYiG4KXJJB" Content-Disposition: inline --EVx8OOgYiG4KXJJB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable * Michael Paquier (michael.paquier@gmail.com) wrote: > On Sat, May 13, 2017 at 3:22 AM, Tom Lane wrote: > > tureba@gmail.com writes: > >> It seems that pg_dump -c fails when the database doesn't have a 'publi= c' > >> 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 no= t exist > >> LINE 1: ...::regclass AND pip.objsubid =3D 0 AND pip.objoid <> 'public= '::... > >> ^ > > > >> This seems to have started in commit 330b84d8c4 > > > > Yeah, this is not cool. Stephen? >=20 > 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 =3D '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 =3D '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 --EVx8OOgYiG4KXJJB Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="fix_pg_dump_no_public_schema_v1-master.patch" Content-Transfer-Encoding: quoted-printable diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c new file mode 100644 index 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"); =20 appendPQExpBuffer(query,") "); =20 --- 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 =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 new file mode 100644 index 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 =3D ( *** 103,108 **** --- 103,125 ---- '-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', ], }, # Do not use --no-sync to give test coverage for data sync. defaults_custom_format =3D> { test_key =3D> 'defaults', *************** qr/CREATE TRANSFORM FOR integer LANGUAGE *** 4513,4518 **** --- 4530,4572 ---- pg_dumpall_globals_clean =3D> 1, test_schema_plus_blobs =3D> 1, }, }, =20 + 'CREATE SCHEMA public (none exists)' =3D> { + all_runs =3D> 1, + database =3D> 'regress_pg_dump_test', + regexp =3D> qr/^DROP SCHEMA public;/m, + like =3D> { }, + unlike =3D> { defaults_no_public =3D> 1, + defaults_no_public_clean =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, }, }, +=20 'CREATE SCHEMA dump_test' =3D> { all_runs =3D> 1, catch_all =3D> 'CREATE ... commands', *************** qr/CREATE TRANSFORM FOR integer LANGUAGE *** 5205,5210 **** --- 5259,5274 ---- data_only =3D> 1, section_data =3D> 1, }, }, =20 + 'DROP SCHEMA public' =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, } }, +=20 'DROP EXTENSION plpgsql' =3D> { all_runs =3D> 1, catch_all =3D> 'DROP ... commands', *************** if ($collation_check_stderr !~ /ERROR: / *** 6411,6416 **** --- 6475,6483 ---- $collation_support =3D 1; } =20 + # Create a second database for certain tests to work against + $node->psql('postgres','create database regress_pg_dump_test;'); +=20 # Start with number of command_fails_like()*2 tests below (each # command_fails_like is actually 2 tests) my $num_tests =3D 12; *************** my $num_tests =3D 12; *** 6418,6423 **** --- 6485,6495 ---- foreach my $run (sort keys %pgdump_runs) { my $test_key =3D $run; + my $run_db =3D 'postgres'; +=20 + 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++; *************** 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 =3D 'postgres'; +=20 + if (defined($tests{$test}->{database})) { + $test_db =3D $tests{$test}->{database}; + } +=20 + 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})) { next; *************** plan tests =3D> $num_tests; *** 6483,6489 **** # Set up schemas, tables, etc, to be dumped. =20 # Build up the create statements ! my $create_sql =3D ''; =20 foreach my $test ( sort { --- 6565,6571 ---- # Set up schemas, tables, etc, to be dumped. =20 # Build up the create statements ! my %create_sql =3D (); =20 foreach my $test ( sort { *************** foreach my $test ( *** 6505,6510 **** --- 6587,6598 ---- } } keys %tests) { + my $test_db =3D 'postgres'; +=20 + if (defined($tests{$test}->{database})) { + $test_db =3D $tests{$test}->{database}; + } +=20 if ($tests{$test}->{create_sql}) { # Skip any collation-related commands if there is no collation support *************** foreach my $test ( *** 6513,6524 **** } =20 # Add terminating semicolon ! $create_sql .=3D $tests{$test}->{create_sql} . ";"; } } =20 # Send the combined set of commands to psql ! $node->safe_psql('postgres', $create_sql); =20 ######################################### # Test connecting to a non-existent database --- 6601,6615 ---- } =20 # Add terminating semicolon ! $create_sql{$test_db} .=3D $tests{$test}->{create_sql} . ";"; } } =20 # Send the combined set of commands to psql ! foreach my $db (sort keys %create_sql) ! { ! $node->safe_psql($db, $create_sql{$db}); ! } =20 ######################################### # Test connecting to a non-existent database *************** command_fails_like( *** 6565,6571 **** =20 foreach my $run (sort keys %pgdump_runs) { -=20 my $test_key =3D $run; =20 $node->command_ok(\@{ $pgdump_runs{$run}->{dump_cmd} }, --- 6656,6661 ---- --EVx8OOgYiG4KXJJB-- --K90HjsLT/gE7/naG Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJZGHpoAAoJEO1sijiDR2RVvikP/0RInAl3oz3lgGuk7K1cb1Gr iHrIEY4iX5yA9O7U6/u+idLV6cq9u1MMkYBsc0aN0x7MRzvaiALJdf87CrDBCkA8 3MwJ4KtJfrxAKvbI/aufkakZqfBC4rKVRDj77COnSHSujM/j+lFOUEIouWYuyL1K uqSMdHeuSBpCvUGCOLvR7L0wI0HUgyRvRIKrvGkcWGDwbdsmoRdV6Z++daBjC3hu Nw6U9QHUFl6mHhZlawkLHrK5K9VIaHZ78AcA6OYCWWlFtFvJld8NPPiOOgBgmjZg 6hAeLLBpYBXfgMQ04MQ2YAyrbu4TD0RTHdSqzejLW/U2Hyg8EvNBwNTXJqgqzpxQ raQgwnM1Fgz1ObhS0Pm6WJviDEukSKd44qytr/MTGaA/agn1PV5AtY9B7xrw7+VJ H5OpIIgCBll1tvLZ7G3wyuyx02vUd00bKkHZ2uyVMt4vVvXug5QoeON1uo1V1dvQ P8w5NJH/7GJ43MfY+EMb7XKUc5ZJSA03qQ/3WanN66HIlA0LqNboYplqgifkQ3Tn P1k7qfBkzbzL6b8xQeZ8gWZt7Jht7IdV3/dTHOPyX0xR6v8QGnXFCoY/cs08Dfxu 3XGSCS7D5WKxqvTQRsnf8fkjvdRngK0S1LEtfFrmm0Q4iomQp+6P0B5fUBvxBm92 NPTDT2YwrKFVFHn6YN1y =t7sc -----END PGP SIGNATURE----- --K90HjsLT/gE7/naG--