Skip to content

Commit 5955d93

Browse files
tglsfdctglsfdc
committed
Improve pg_dump's handling of "special" built-in objects.
We had some pretty ad-hoc handling of the public schema and the plpgsql extension, which are both presumed to exist in template0 but might be modified or deleted in the database being dumped. Up to now, by default pg_dump would emit a CREATE EXTENSION IF NOT EXISTS command as well as a COMMENT command for plpgsql. The usefulness of the former is questionable, and the latter caused annoying errors in non-superuser dump/restore scenarios. Let's instead install a rule that built-in extensions (identified by having low-numbered OIDs) are not to be dumped. We were doing it that way already in binary-upgrade mode, so this just makes regular mode behave the same. It remains true that if someone has installed a non-default ACL on the plpgsql language, that will get dumped thanks to the pg_init_privs mechanism. This is more consistent with the handling of built-in objects of other kinds. Also, change the very ad-hoc mechanism that was used to avoid dumping creation and comment commands for the public schema. Instead of hardwiring a test in _printTocEntry(), make use of the DUMP_COMPONENT_ infrastructure to mark that schema up-front about what we want to do with it. This has the visible effect that the public schema won't be mentioned in the output at all, except for updating its ACL if it has a non-default ACL. Previously, while it was normally not mentioned, --clean mode would drop and recreate it, again causing headaches for non-superuser usage. This change likewise makes the public schema less special and more like other built-in objects. If plpgsql, or the public schema, has been removed entirely in the source DB, that situation won't be reproduced in the destination ... but that was true before. Discussion: https://postgr.es/m/29048.1516812451@sss.pgh.pa.us
1 parent 2a5ecb5 commit 5955d93

File tree

3 files changed

+87
-102
lines changed

3 files changed

+87
-102
lines changed

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3453,29 +3453,6 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
34533453
{
34543454
RestoreOptions *ropt = AH->public.ropt;
34553455

3456-
/*
3457-
* Avoid dumping the public schema, as it will already be created ...
3458-
* unless we are using --clean mode (and *not* --create mode), in which
3459-
* case we've previously issued a DROP for it so we'd better recreate it.
3460-
*
3461-
* Likewise for its comment, if any. (We could try issuing the COMMENT
3462-
* command anyway; but it'd fail if the restore is done as non-super-user,
3463-
* so let's not.)
3464-
*
3465-
* XXX it looks pretty ugly to hard-wire the public schema like this, but
3466-
* it sits in a sort of no-mans-land between being a system object and a
3467-
* user object, so it really is special in a way.
3468-
*/
3469-
if (!(ropt->dropSchema && !ropt->createDB))
3470-
{
3471-
if (strcmp(te->desc, "SCHEMA") == 0 &&
3472-
strcmp(te->tag, "public") == 0)
3473-
return;
3474-
if (strcmp(te->desc, "COMMENT") == 0 &&
3475-
strcmp(te->tag, "SCHEMA public") == 0)
3476-
return;
3477-
}
3478-
34793456
/* Select owner, schema, and tablespace as necessary */
34803457
_becomeOwner(AH, te);
34813458
_selectOutputSchema(AH, te->namespace);

‎src/bin/pg_dump/pg_dump.c

Lines changed: 35 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,6 +1407,19 @@ selectDumpableNamespace(NamespaceInfo *nsinfo, Archive *fout)
14071407
/* Other system schemas don't get dumped */
14081408
nsinfo->dobj.dump_contains = nsinfo->dobj.dump = DUMP_COMPONENT_NONE;
14091409
}
1410+
else if (strcmp(nsinfo->dobj.name, "public") == 0)
1411+
{
1412+
/*
1413+
* The public schema is a strange beast that sits in a sort of
1414+
* no-mans-land between being a system object and a user object. We
1415+
* don't want to dump creation or comment commands for it, because
1416+
* that complicates matters for non-superuser use of pg_dump. But we
1417+
* should dump any ACL changes that have occurred for it, and of
1418+
* course we should dump contained objects.
1419+
*/
1420+
nsinfo->dobj.dump = DUMP_COMPONENT_ACL;
1421+
nsinfo->dobj.dump_contains = DUMP_COMPONENT_ALL;
1422+
}
14101423
else
14111424
nsinfo->dobj.dump_contains = nsinfo->dobj.dump = DUMP_COMPONENT_ALL;
14121425

@@ -1617,21 +1630,21 @@ selectDumpableAccessMethod(AccessMethodInfo *method, Archive *fout)
16171630
* selectDumpableExtension: policy-setting subroutine
16181631
* Mark an extension as to be dumped or not
16191632
*
1620-
* Normally, we dump all extensions, or none of them if include_everything
1621-
* is false (i.e., a --schema or --table switch was given). However, in
1622-
* binary-upgrade mode it's necessary to skip built-in extensions, since we
1633+
* Built-in extensions should be skipped except for checking ACLs, since we
16231634
* assume those will already be installed in the target database. We identify
16241635
* such extensions by their having OIDs in the range reserved for initdb.
1636+
* We dump all user-added extensions by default, or none of them if
1637+
* include_everything is false (i.e., a --schema or --table switch was given).
16251638
*/
16261639
static void
16271640
selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
16281641
{
16291642
/*
1630-
* Use DUMP_COMPONENT_ACL for from-initdb extensions, to allow users to
1631-
* change permissions on those objects, if they wish to, and have those
1632-
* changes preserved.
1643+
* Use DUMP_COMPONENT_ACL for built-in extensions, to allow users to
1644+
* change permissions on their member objects, if they wish to, and have
1645+
* those changes preserved.
16331646
*/
1634-
if (dopt->binary_upgrade && extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
1647+
if (extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
16351648
extinfo->dobj.dump = extinfo->dobj.dump_contains = DUMP_COMPONENT_ACL;
16361649
else
16371650
extinfo->dobj.dump = extinfo->dobj.dump_contains =
@@ -4435,29 +4448,6 @@ getNamespaces(Archive *fout, int *numNamespaces)
44354448
init_acl_subquery->data,
44364449
init_racl_subquery->data);
44374450

4438-
/*
4439-
* When we are doing a 'clean' run, we will be dropping and recreating
4440-
* the 'public' schema (the only object which has that kind of
4441-
* treatment in the backend and which has an entry in pg_init_privs)
4442-
* and therefore we should not consider any initial privileges in
4443-
* pg_init_privs in that case.
4444-
*
4445-
* See pg_backup_archiver.c:_printTocEntry() for the details on why
4446-
* the public schema is special in this regard.
4447-
*
4448-
* Note that if the public schema is dropped and re-created, this is
4449-
* essentially a no-op because the new public schema won't have an
4450-
* entry in pg_init_privs anyway, as the entry will be removed when
4451-
* the public schema is dropped.
4452-
*
4453-
* Further, we have to handle the case where the public schema does
4454-
* not exist at all.
4455-
*/
4456-
if (dopt->outputClean)
4457-
appendPQExpBuffer(query, " AND pip.objoid <> "
4458-
"coalesce((select oid from pg_namespace "
4459-
"where nspname = 'public'),0)");
4460-
44614451
appendPQExpBuffer(query, ") ");
44624452

44634453
destroyPQExpBuffer(acl_subquery);
@@ -9945,20 +9935,28 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
99459935
if (!dopt->binary_upgrade)
99469936
{
99479937
/*
9948-
* In a regular dump, we use IF NOT EXISTS so that there isn't a
9949-
* problem if the extension already exists in the target database;
9950-
* this is essential for installed-by-default extensions such as
9951-
* plpgsql.
9938+
* In a regular dump, we simply create the extension, intentionally
9939+
* not specifying a version, so that the destination installation's
9940+
* default version is used.
99529941
*
9953-
* In binary-upgrade mode, that doesn't work well, so instead we skip
9954-
* built-in extensions based on their OIDs; see
9955-
* selectDumpableExtension.
9942+
* Use of IF NOT EXISTS here is unlike our behavior for other object
9943+
* types; but there are various scenarios in which it's convenient to
9944+
* manually create the desired extension before restoring, so we
9945+
* prefer to allow it to exist already.
99569946
*/
99579947
appendPQExpBuffer(q, "CREATE EXTENSION IF NOT EXISTS %s WITH SCHEMA %s;\n",
99589948
qextname, fmtId(extinfo->namespace));
99599949
}
99609950
else
99619951
{
9952+
/*
9953+
* In binary-upgrade mode, it's critical to reproduce the state of the
9954+
* database exactly, so our procedure is to create an empty extension,
9955+
* restore all the contained objects normally, and add them to the
9956+
* extension one by one. This function performs just the first of
9957+
* those steps. binary_upgrade_extension_member() takes care of
9958+
* adding member objects as they're created.
9959+
*/
99629960
int i;
99639961
int n;
99649962

@@ -9968,8 +9966,6 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
99689966
* We unconditionally create the extension, so we must drop it if it
99699967
* exists. This could happen if the user deleted 'plpgsql' and then
99709968
* readded it, causing its oid to be greater than g_last_builtin_oid.
9971-
* The g_last_builtin_oid test was kept to avoid repeatedly dropping
9972-
* and recreating extensions like 'plpgsql'.
99739969
*/
99749970
appendPQExpBuffer(q, "DROP EXTENSION IF EXISTS %s;\n", qextname);
99759971

‎src/bin/pg_dump/t/002_pg_dump.pl

Lines changed: 52 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1548,30 +1548,31 @@
15481548
all_runs => 1,
15491549
catch_all => 'COMMENT commands',
15501550
regexp => qr/^COMMENT ON EXTENSION plpgsql IS .*;/m,
1551-
like => {
1551+
# this shouldn't ever get emitted anymore
1552+
like => {},
1553+
unlike => {
1554+
binary_upgrade => 1,
15521555
clean => 1,
15531556
clean_if_exists => 1,
1557+
column_inserts => 1,
15541558
createdb => 1,
1559+
data_only => 1,
15551560
defaults => 1,
15561561
exclude_dump_test_schema => 1,
15571562
exclude_test_table => 1,
15581563
exclude_test_table_data => 1,
15591564
no_blobs => 1,
1560-
no_privs => 1,
15611565
no_owner => 1,
1566+
no_privs => 1,
1567+
only_dump_test_schema => 1,
1568+
only_dump_test_table => 1,
15621569
pg_dumpall_dbprivs => 1,
1570+
role => 1,
15631571
schema_only => 1,
1572+
section_post_data => 1,
15641573
section_pre_data => 1,
1565-
with_oids => 1, },
1566-
unlike => {
1567-
binary_upgrade => 1,
1568-
column_inserts => 1,
1569-
data_only => 1,
1570-
only_dump_test_schema => 1,
1571-
only_dump_test_table => 1,
1572-
role => 1,
1573-
section_post_data => 1,
1574-
test_schema_plus_blobs => 1, }, },
1574+
test_schema_plus_blobs => 1,
1575+
with_oids => 1, }, },
15751576

15761577
'COMMENT ON TABLE dump_test.test_table' => {
15771578
all_runs => 1,
@@ -2751,33 +2752,34 @@
27512752
regexp => qr/^
27522753
\QCREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog;\E
27532754
/xm,
2754-
like => {
2755+
# this shouldn't ever get emitted anymore
2756+
like => {},
2757+
unlike => {
2758+
binary_upgrade => 1,
27552759
clean => 1,
27562760
clean_if_exists => 1,
2761+
column_inserts => 1,
27572762
createdb => 1,
2763+
data_only => 1,
27582764
defaults => 1,
27592765
exclude_dump_test_schema => 1,
27602766
exclude_test_table => 1,
27612767
exclude_test_table_data => 1,
27622768
no_blobs => 1,
2763-
no_privs => 1,
27642769
no_owner => 1,
2765-
pg_dumpall_dbprivs => 1,
2766-
schema_only => 1,
2767-
section_pre_data => 1,
2768-
with_oids => 1, },
2769-
unlike => {
2770-
binary_upgrade => 1,
2771-
column_inserts => 1,
2772-
data_only => 1,
2770+
no_privs => 1,
27732771
only_dump_test_schema => 1,
27742772
only_dump_test_table => 1,
2773+
pg_dumpall_dbprivs => 1,
27752774
pg_dumpall_globals => 1,
27762775
pg_dumpall_globals_clean => 1,
27772776
role => 1,
2777+
schema_only => 1,
27782778
section_data => 1,
27792779
section_post_data => 1,
2780-
test_schema_plus_blobs => 1, }, },
2780+
section_pre_data => 1,
2781+
test_schema_plus_blobs => 1,
2782+
with_oids => 1, }, },
27812783

27822784
'CREATE AGGREGATE dump_test.newavg' => {
27832785
all_runs => 1,
@@ -4565,11 +4567,12 @@
45654567
all_runs => 1,
45664568
catch_all => 'CREATE ... commands',
45674569
regexp => qr/^CREATE SCHEMA public;/m,
4568-
like => {
4569-
clean => 1,
4570-
clean_if_exists => 1, },
4570+
# this shouldn't ever get emitted anymore
4571+
like => {},
45714572
unlike => {
45724573
binary_upgrade => 1,
4574+
clean => 1,
4575+
clean_if_exists => 1,
45734576
createdb => 1,
45744577
defaults => 1,
45754578
exclude_test_table => 1,
@@ -5395,26 +5398,32 @@
53955398
all_runs => 1,
53965399
catch_all => 'DROP ... commands',
53975400
regexp => qr/^DROP SCHEMA public;/m,
5398-
like => { clean => 1 },
5401+
# this shouldn't ever get emitted anymore
5402+
like => {},
53995403
unlike => {
5404+
clean => 1,
54005405
clean_if_exists => 1,
54015406
pg_dumpall_globals_clean => 1, }, },
54025407
54035408
'DROP SCHEMA IF EXISTS public' => {
54045409
all_runs => 1,
54055410
catch_all => 'DROP ... commands',
54065411
regexp => qr/^DROP SCHEMA IF EXISTS public;/m,
5407-
like => { clean_if_exists => 1 },
5412+
# this shouldn't ever get emitted anymore
5413+
like => {},
54085414
unlike => {
54095415
clean => 1,
5416+
clean_if_exists => 1,
54105417
pg_dumpall_globals_clean => 1, }, },
54115418
54125419
'DROP EXTENSION plpgsql' => {
54135420
all_runs => 1,
54145421
catch_all => 'DROP ... commands',
54155422
regexp => qr/^DROP EXTENSION plpgsql;/m,
5416-
like => { clean => 1, },
5423+
# this shouldn't ever get emitted anymore
5424+
like => {},
54175425
unlike => {
5426+
clean => 1,
54185427
clean_if_exists => 1,
54195428
pg_dumpall_globals_clean => 1, }, },
54205429
@@ -5494,9 +5503,11 @@
54945503
all_runs => 1,
54955504
catch_all => 'DROP ... commands',
54965505
regexp => qr/^DROP EXTENSION IF EXISTS plpgsql;/m,
5497-
like => { clean_if_exists => 1, },
5506+
# this shouldn't ever get emitted anymore
5507+
like => {},
54985508
unlike => {
54995509
clean => 1,
5510+
clean_if_exists => 1,
55005511
pg_dumpall_globals_clean => 1, }, },
55015512
55025513
'DROP FUNCTION IF EXISTS dump_test.pltestlang_call_handler()' => {
@@ -6264,11 +6275,12 @@
62646275
\Q--\E\n\n
62656276
\QGRANT USAGE ON SCHEMA public TO PUBLIC;\E
62666277
/xm,
6267-
like => {
6268-
clean => 1,
6269-
clean_if_exists => 1, },
6278+
# this shouldn't ever get emitted anymore
6279+
like => {},
62706280
unlike => {
62716281
binary_upgrade => 1,
6282+
clean => 1,
6283+
clean_if_exists => 1,
62726284
createdb => 1,
62736285
defaults => 1,
62746286
exclude_dump_test_schema => 1,
@@ -6537,6 +6549,8 @@
65376549
/xm,
65386550
like => {
65396551
binary_upgrade => 1,
6552+
clean => 1,
6553+
clean_if_exists => 1,
65406554
createdb => 1,
65416555
defaults => 1,
65426556
exclude_dump_test_schema => 1,
@@ -6549,8 +6563,6 @@
65496563
section_pre_data => 1,
65506564
with_oids => 1, },
65516565
unlike => {
6552-
clean => 1,
6553-
clean_if_exists => 1,
65546566
only_dump_test_schema => 1,
65556567
only_dump_test_table => 1,
65566568
pg_dumpall_globals_clean => 1,
@@ -6576,18 +6588,18 @@
65766588
exclude_test_table_data => 1,
65776589
no_blobs => 1,
65786590
no_owner => 1,
6591+
only_dump_test_schema => 1,
6592+
only_dump_test_table => 1,
65796593
pg_dumpall_dbprivs => 1,
6594+
role => 1,
65806595
schema_only => 1,
65816596
section_pre_data => 1,
6597+
test_schema_plus_blobs => 1,
65826598
with_oids => 1, },
65836599
unlike => {
6584-
only_dump_test_schema => 1,
6585-
only_dump_test_table => 1,
65866600
pg_dumpall_globals_clean => 1,
6587-
role => 1,
65886601
section_data => 1,
6589-
section_post_data => 1,
6590-
test_schema_plus_blobs => 1, }, },
6602+
section_post_data => 1, }, },
65916603
65926604
'REVOKE commands' => { # catch-all for REVOKE commands
65936605
all_runs => 0, # catch-all

0 commit comments

Comments
 (0)