Skip to content

Commit 0d4e6ed

Browse files
tglsfdctglsfdc
committed
Clean up some aspects of pg_dump/pg_restore item-selection logic.
Ensure that CREATE DATABASE and related commands are issued when, and only when, --create is specified. Previously there were scenarios where using selective-dump switches would prevent --create from having any effect. For example, it would fail to do anything in pg_restore if the archive file had been made by a selective dump, because there would be no TOC entry for the database. Since we don't issue \connect either if we don't issue CREATE DATABASE, this could result in unexpectedly restoring objects into the wrong database. Also fix pg_restore's selective restore logic so that when an object is selected to be restored, we also restore its ACL, comment, and security label if any. Previously there was no way to get the latter properties except through tedious mucking about with a -L file. If, for some reason, you don't want these properties, you can match the old behavior by adding --no-acl etc. While at it, try to make _tocEntryRequired() a little better organized and better documented. Discussion: https://postgr.es/m/32668.1516848577@sss.pgh.pa.us
1 parent 05fb5d6 commit 0d4e6ed

File tree

3 files changed

+153
-89
lines changed

3 files changed

+153
-89
lines changed

‎doc/src/sgml/ref/pg_restore.sgml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,11 @@
446446
flag of <application>pg_dump</application>. There is not currently
447447
any provision for wild-card matching in <application>pg_restore</application>,
448448
nor can you include a schema name within its <option>-t</option>.
449+
And, while <application>pg_dump</application>'s <option>-t</option>
450+
flag will also dump subsidiary objects (such as indexes) of the
451+
selected table(s),
452+
<application>pg_restore</application>'s <option>-t</option>
453+
flag does not include such subsidiary objects.
449454
</para>
450455
</note>
451456

@@ -564,7 +569,7 @@
564569
<listitem>
565570
<para>
566571
Use conditional commands (i.e. add an <literal>IF EXISTS</literal>
567-
clause) when cleaning database objects. This option is not valid
572+
clause) to drop database objects. This option is not valid
568573
unless <option>--clean</option> is also specified.
569574
</para>
570575
</listitem>

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 139 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ static void _selectOutputSchema(ArchiveHandle *AH, const char *schemaName);
7070
static void _selectTablespace(ArchiveHandle *AH, const char *tablespace);
7171
static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te);
7272
static void processStdStringsEntry(ArchiveHandle *AH, TocEntry *te);
73-
static teReqs _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt);
73+
static teReqs _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH);
7474
static RestorePass _tocEntryRestorePass(TocEntry *te);
7575
static bool _tocEntryIsACL(TocEntry *te);
7676
static void _disableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te);
@@ -312,7 +312,7 @@ ProcessArchiveRestoreOptions(Archive *AHX)
312312
if (te->section != SECTION_NONE)
313313
curSection = te->section;
314314

315-
te->reqs = _tocEntryRequired(te, curSection, ropt);
315+
te->reqs = _tocEntryRequired(te, curSection, AH);
316316
}
317317

318318
/* Enforce strict names checking */
@@ -488,22 +488,15 @@ RestoreArchive(Archive *AHX)
488488
* In createDB mode, issue a DROP *only* for the database as a
489489
* whole. Issuing drops against anything else would be wrong,
490490
* because at this point we're connected to the wrong database.
491-
* Conversely, if we're not in createDB mode, we'd better not
492-
* issue a DROP against the database at all. (The DATABASE
493-
* PROPERTIES entry, if any, works like the DATABASE entry.)
491+
* (The DATABASE PROPERTIES entry, if any, should be treated like
492+
* the DATABASE entry.)
494493
*/
495494
if (ropt->createDB)
496495
{
497496
if (strcmp(te->desc, "DATABASE") != 0 &&
498497
strcmp(te->desc, "DATABASE PROPERTIES") != 0)
499498
continue;
500499
}
501-
else
502-
{
503-
if (strcmp(te->desc, "DATABASE") == 0 ||
504-
strcmp(te->desc, "DATABASE PROPERTIES") == 0)
505-
continue;
506-
}
507500

508501
/* Otherwise, drop anything that's selected and has a dropStmt */
509502
if (((te->reqs & (REQ_SCHEMA | REQ_DATA)) != 0) && te->dropStmt)
@@ -752,25 +745,6 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
752745

753746
AH->currentTE = te;
754747

755-
/* Work out what, if anything, we want from this entry */
756-
reqs = te->reqs;
757-
758-
/*
759-
* Ignore DATABASE and related entries unless createDB is specified. We
760-
* must check this here, not in _tocEntryRequired, because !createDB
761-
* should not prevent emitting these entries to an archive file.
762-
*/
763-
if (!ropt->createDB &&
764-
(strcmp(te->desc, "DATABASE") == 0 ||
765-
strcmp(te->desc, "DATABASE PROPERTIES") == 0 ||
766-
(strcmp(te->desc, "ACL") == 0 &&
767-
strncmp(te->tag, "DATABASE ", 9) == 0) ||
768-
(strcmp(te->desc, "COMMENT") == 0 &&
769-
strncmp(te->tag, "DATABASE ", 9) == 0) ||
770-
(strcmp(te->desc, "SECURITY LABEL") == 0 &&
771-
strncmp(te->tag, "DATABASE ", 9) == 0)))
772-
reqs = 0;
773-
774748
/* Dump any relevant dump warnings to stderr */
775749
if (!ropt->suppressDumpWarnings && strcmp(te->desc, "WARNING") == 0)
776750
{
@@ -780,6 +754,9 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
780754
write_msg(modulename, "warning from original dump file: %s\n", te->copyStmt);
781755
}
782756

757+
/* Work out what, if anything, we want from this entry */
758+
reqs = te->reqs;
759+
783760
defnDumped = false;
784761

785762
/*
@@ -1191,7 +1168,7 @@ PrintTOCSummary(Archive *AHX)
11911168
if (te->section != SECTION_NONE)
11921169
curSection = te->section;
11931170
if (ropt->verbose ||
1194-
(_tocEntryRequired(te, curSection, ropt) & (REQ_SCHEMA | REQ_DATA)) != 0)
1171+
(_tocEntryRequired(te, curSection, AH) & (REQ_SCHEMA | REQ_DATA)) != 0)
11951172
{
11961173
char *sanitized_name;
11971174
char *sanitized_schema;
@@ -2824,16 +2801,42 @@ StrictNamesCheck(RestoreOptions *ropt)
28242801
}
28252802
}
28262803

2804+
/*
2805+
* Determine whether we want to restore this TOC entry.
2806+
*
2807+
* Returns 0 if entry should be skipped, or some combination of the
2808+
* REQ_SCHEMA and REQ_DATA bits if we want to restore schema and/or data
2809+
* portions of this TOC entry, or REQ_SPECIAL if it's a special entry.
2810+
*/
28272811
static teReqs
2828-
_tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
2812+
_tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH)
28292813
{
28302814
teReqs res = REQ_SCHEMA | REQ_DATA;
2815+
RestoreOptions *ropt = AH->public.ropt;
28312816

28322817
/* ENCODING and STDSTRINGS items are treated specially */
28332818
if (strcmp(te->desc, "ENCODING") == 0 ||
28342819
strcmp(te->desc, "STDSTRINGS") == 0)
28352820
return REQ_SPECIAL;
28362821

2822+
/*
2823+
* DATABASE and DATABASE PROPERTIES also have a special rule: they are
2824+
* restored in createDB mode, and not restored otherwise, independently of
2825+
* all else.
2826+
*/
2827+
if (strcmp(te->desc, "DATABASE") == 0 ||
2828+
strcmp(te->desc, "DATABASE PROPERTIES") == 0)
2829+
{
2830+
if (ropt->createDB)
2831+
return REQ_SCHEMA;
2832+
else
2833+
return 0;
2834+
}
2835+
2836+
/*
2837+
* Process exclusions that affect certain classes of TOC entries.
2838+
*/
2839+
28372840
/* If it's an ACL, maybe ignore it */
28382841
if (ropt->aclsSkip && _tocEntryIsACL(te))
28392842
return 0;
@@ -2842,11 +2845,11 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
28422845
if (ropt->no_publications && strcmp(te->desc, "PUBLICATION") == 0)
28432846
return 0;
28442847

2845-
/* If it's security labels, maybe ignore it */
2848+
/* If it's a security label, maybe ignore it */
28462849
if (ropt->no_security_labels && strcmp(te->desc, "SECURITY LABEL") == 0)
28472850
return 0;
28482851

2849-
/* If it's a subcription, maybe ignore it */
2852+
/* If it's a subscription, maybe ignore it */
28502853
if (ropt->no_subscriptions && strcmp(te->desc, "SUBSCRIPTION") == 0)
28512854
return 0;
28522855

@@ -2870,65 +2873,118 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
28702873
return 0;
28712874
}
28722875

2873-
/* Check options for selective dump/restore */
2874-
if (ropt->schemaNames.head != NULL)
2875-
{
2876-
/* If no namespace is specified, it means all. */
2877-
if (!te->namespace)
2878-
return 0;
2879-
if (!(simple_string_list_member(&ropt->schemaNames, te->namespace)))
2880-
return 0;
2881-
}
2882-
2883-
if (ropt->schemaExcludeNames.head != NULL &&
2884-
te->namespace &&
2885-
simple_string_list_member(&ropt->schemaExcludeNames, te->namespace))
2876+
/* Ignore it if rejected by idWanted[] (cf. SortTocFromFile) */
2877+
if (ropt->idWanted && !ropt->idWanted[te->dumpId - 1])
28862878
return 0;
28872879

2888-
if (ropt->selTypes)
2880+
/*
2881+
* Check options for selective dump/restore.
2882+
*/
2883+
if (strcmp(te->desc, "ACL") == 0 ||
2884+
strcmp(te->desc, "COMMENT") == 0 ||
2885+
strcmp(te->desc, "SECURITY LABEL") == 0)
28892886
{
2890-
if (strcmp(te->desc, "TABLE") == 0 ||
2891-
strcmp(te->desc, "TABLE DATA") == 0 ||
2892-
strcmp(te->desc, "VIEW") == 0 ||
2893-
strcmp(te->desc, "FOREIGN TABLE") == 0 ||
2894-
strcmp(te->desc, "MATERIALIZED VIEW") == 0 ||
2895-
strcmp(te->desc, "MATERIALIZED VIEW DATA") == 0 ||
2896-
strcmp(te->desc, "SEQUENCE") == 0 ||
2897-
strcmp(te->desc, "SEQUENCE SET") == 0)
2887+
/* Database properties react to createDB, not selectivity options. */
2888+
if (strncmp(te->tag, "DATABASE ", 9) == 0)
28982889
{
2899-
if (!ropt->selTable)
2900-
return 0;
2901-
if (ropt->tableNames.head != NULL && (!(simple_string_list_member(&ropt->tableNames, te->tag))))
2890+
if (!ropt->createDB)
29022891
return 0;
29032892
}
2904-
else if (strcmp(te->desc, "INDEX") == 0)
2893+
else if (ropt->schemaNames.head != NULL ||
2894+
ropt->schemaExcludeNames.head != NULL ||
2895+
ropt->selTypes)
29052896
{
2906-
if (!ropt->selIndex)
2907-
return 0;
2908-
if (ropt->indexNames.head != NULL && (!(simple_string_list_member(&ropt->indexNames, te->tag))))
2897+
/*
2898+
* In a selective dump/restore, we want to restore these dependent
2899+
* TOC entry types only if their parent object is being restored.
2900+
* Without selectivity options, we let through everything in the
2901+
* archive. Note there may be such entries with no parent, eg
2902+
* non-default ACLs for built-in objects.
2903+
*
2904+
* This code depends on the parent having been marked already,
2905+
* which should be the case; if it isn't, perhaps due to
2906+
* SortTocFromFile rearrangement, skipping the dependent entry
2907+
* seems prudent anyway.
2908+
*
2909+
* Ideally we'd handle, eg, table CHECK constraints this way too.
2910+
* But it's hard to tell which of their dependencies is the one to
2911+
* consult.
2912+
*/
2913+
if (te->nDeps != 1 ||
2914+
TocIDRequired(AH, te->dependencies[0]) == 0)
29092915
return 0;
29102916
}
2911-
else if (strcmp(te->desc, "FUNCTION") == 0 ||
2912-
strcmp(te->desc, "PROCEDURE") == 0)
2917+
}
2918+
else
2919+
{
2920+
/* Apply selective-restore rules for standalone TOC entries. */
2921+
if (ropt->schemaNames.head != NULL)
29132922
{
2914-
if (!ropt->selFunction)
2923+
/* If no namespace is specified, it means all. */
2924+
if (!te->namespace)
29152925
return 0;
2916-
if (ropt->functionNames.head != NULL && (!(simple_string_list_member(&ropt->functionNames, te->tag))))
2926+
if (!simple_string_list_member(&ropt->schemaNames, te->namespace))
29172927
return 0;
29182928
}
2919-
else if (strcmp(te->desc, "TRIGGER") == 0)
2929+
2930+
if (ropt->schemaExcludeNames.head != NULL &&
2931+
te->namespace &&
2932+
simple_string_list_member(&ropt->schemaExcludeNames, te->namespace))
2933+
return 0;
2934+
2935+
if (ropt->selTypes)
29202936
{
2921-
if (!ropt->selTrigger)
2922-
return 0;
2923-
if (ropt->triggerNames.head != NULL && (!(simple_string_list_member(&ropt->triggerNames, te->tag))))
2937+
if (strcmp(te->desc, "TABLE") == 0 ||
2938+
strcmp(te->desc, "TABLE DATA") == 0 ||
2939+
strcmp(te->desc, "VIEW") == 0 ||
2940+
strcmp(te->desc, "FOREIGN TABLE") == 0 ||
2941+
strcmp(te->desc, "MATERIALIZED VIEW") == 0 ||
2942+
strcmp(te->desc, "MATERIALIZED VIEW DATA") == 0 ||
2943+
strcmp(te->desc, "SEQUENCE") == 0 ||
2944+
strcmp(te->desc, "SEQUENCE SET") == 0)
2945+
{
2946+
if (!ropt->selTable)
2947+
return 0;
2948+
if (ropt->tableNames.head != NULL &&
2949+
!simple_string_list_member(&ropt->tableNames, te->tag))
2950+
return 0;
2951+
}
2952+
else if (strcmp(te->desc, "INDEX") == 0)
2953+
{
2954+
if (!ropt->selIndex)
2955+
return 0;
2956+
if (ropt->indexNames.head != NULL &&
2957+
!simple_string_list_member(&ropt->indexNames, te->tag))
2958+
return 0;
2959+
}
2960+
else if (strcmp(te->desc, "FUNCTION") == 0 ||
2961+
strcmp(te->desc, "AGGREGATE") == 0 ||
2962+
strcmp(te->desc, "PROCEDURE") == 0)
2963+
{
2964+
if (!ropt->selFunction)
2965+
return 0;
2966+
if (ropt->functionNames.head != NULL &&
2967+
!simple_string_list_member(&ropt->functionNames, te->tag))
2968+
return 0;
2969+
}
2970+
else if (strcmp(te->desc, "TRIGGER") == 0)
2971+
{
2972+
if (!ropt->selTrigger)
2973+
return 0;
2974+
if (ropt->triggerNames.head != NULL &&
2975+
!simple_string_list_member(&ropt->triggerNames, te->tag))
2976+
return 0;
2977+
}
2978+
else
29242979
return 0;
29252980
}
2926-
else
2927-
return 0;
29282981
}
29292982

29302983
/*
2931-
* Check if we had a dataDumper. Indicates if the entry is schema or data
2984+
* Determine whether the TOC entry contains schema and/or data components,
2985+
* and mask off inapplicable REQ bits. If it had a dataDumper, assume
2986+
* it's both schema and data. Otherwise it's probably schema-only, but
2987+
* there are exceptions.
29322988
*/
29332989
if (!te->hadDumper)
29342990
{
@@ -2952,6 +3008,10 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
29523008
res = res & ~REQ_DATA;
29533009
}
29543010

3011+
/* If there's no definition command, there's no schema component */
3012+
if (!te->defn || !te->defn[0])
3013+
res = res & ~REQ_SCHEMA;
3014+
29553015
/*
29563016
* Special case: <Init> type with <Max OID> tag; this is obsolete and we
29573017
* always ignore it.
@@ -2963,12 +3023,12 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
29633023
if (ropt->schemaOnly)
29643024
{
29653025
/*
2966-
* The sequence_data option overrides schema-only for SEQUENCE SET.
3026+
* The sequence_data option overrides schemaOnly for SEQUENCE SET.
29673027
*
2968-
* In binary-upgrade mode, even with schema-only set, we do not mask
2969-
* out large objects. Only large object definitions, comments and
2970-
* other information should be generated in binary-upgrade mode (not
2971-
* the actual data).
3028+
* In binary-upgrade mode, even with schemaOnly set, we do not mask
3029+
* out large objects. (Only large object definitions, comments and
3030+
* other metadata should be generated in binary-upgrade mode, not the
3031+
* actual data, but that need not concern us here.)
29723032
*/
29733033
if (!(ropt->sequence_data && strcmp(te->desc, "SEQUENCE SET") == 0) &&
29743034
!(ropt->binary_upgrade &&
@@ -2986,14 +3046,6 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
29863046
if (ropt->dataOnly)
29873047
res = res & REQ_DATA;
29883048

2989-
/* Mask it if we don't have a schema contribution */
2990-
if (!te->defn || strlen(te->defn) == 0)
2991-
res = res & ~REQ_SCHEMA;
2992-
2993-
/* Finally, if there's a per-ID filter, limit based on that as well */
2994-
if (ropt->idWanted && !ropt->idWanted[te->dumpId - 1])
2995-
return 0;
2996-
29973049
return res;
29983050
}
29993051

‎src/bin/pg_dump/pg_dump.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,13 @@ main(int argc, char **argv)
631631
compressLevel = 0;
632632
#endif
633633

634+
/*
635+
* If emitting an archive format, we always want to emit a DATABASE item,
636+
* in case --create is specified at pg_restore time.
637+
*/
638+
if (!plainText)
639+
dopt.outputCreateDB = 1;
640+
634641
/*
635642
* On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
636643
* parallel jobs because that's the maximum limit for the
@@ -841,7 +848,7 @@ main(int argc, char **argv)
841848
dumpStdStrings(fout);
842849

843850
/* The database items are always next, unless we don't want them at all */
844-
if (dopt.include_everything && !dopt.dataOnly)
851+
if (dopt.outputCreateDB)
845852
dumpDatabase(fout);
846853

847854
/* Now the rearrangeable objects. */

0 commit comments

Comments
 (0)