patch: option --if-exists for pg_dump
Hello
I am sending a rebased patch.
Now dump generated with --if-exists option is readable by pg_restore
Regards
Pavel
Attachments:
pg_dump-if-exists.patchtext/x-patch; charset=US-ASCII; name=pg_dump-if-exists.patchDownload
commit 19f21165343b1aaa6cc21d381b84e3c0ce6e3b46
Author: Pavel Stehule <pavel.stehule@gooddata.com>
Date: Fri Dec 13 14:02:46 2013 +0100
--if-exists option for pg_dump
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 8d45f24..18f7346 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -650,6 +650,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It use conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 5c6a101..7e5134e 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -270,6 +270,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It use conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 6927968..83f7216 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -113,6 +113,7 @@ typedef struct _restoreOptions
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 63a8009..23dc77e 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -55,7 +55,8 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt,
const int compression, ArchiveMode mode, SetupWorkerPtr setupWorkerPtr);
static void _getObjectDescription(PQExpBuffer buf, TocEntry *te,
ArchiveHandle *AH);
-static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass);
+static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt,
+ bool isData, bool acl_pass);
static char *replace_line_endings(const char *str);
static void _doSetFixedOutputState(ArchiveHandle *AH);
static void _doSetSessionAuth(ArchiveHandle *AH, const char *user);
@@ -234,6 +235,7 @@ RestoreArchive(Archive *AHX)
bool parallel_mode;
TocEntry *te;
OutputContext sav;
+
AH->stage = STAGE_INITIALIZING;
@@ -2938,9 +2940,35 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
- /* Chop "DROP " off the front and make a modifiable copy */
- char *first = pg_strdup(te->dropStmt + 5);
- char *last;
+ char *first;
+ char *last;
+
+ /* try to search IF EXISTS in DROP command */
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
+ {
+ char buffer[40];
+ size_t l;
+
+ /* IF EXISTS clause should be optional, check it*/
+ snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", type);
+ l = strlen(buffer);
+
+ if (strncmp(te->dropStmt, buffer, l) == 0)
+ {
+ /* append command type to target type */
+ appendPQExpBufferStr(buf, type);
+
+ /* skip first n chars, and create a modifieble copy */
+ first = pg_strdup(te->dropStmt + l);
+ }
+ else
+ /* DROP IF EXISTS pattern is not appliable on dropStmt */
+ first = pg_strdup(te->dropStmt + 5);
+ }
+
+ else
+ /* IF EXISTS clause was not used, simple solution */
+ first = pg_strdup(te->dropStmt + 5);
/* point to last character in string */
last = first + strlen(first) - 1;
@@ -2961,7 +2989,8 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
}
static void
-_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass)
+_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData,
+ bool acl_pass)
{
/* ACLs are dumped only during acl pass */
if (acl_pass)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 224e8cb..0f602c9 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -140,6 +140,7 @@ static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
static int serializable_deferrable = 0;
+static int if_exists = 0;
static void help(const char *progname);
@@ -346,6 +347,7 @@ main(int argc, char **argv)
{"attribute-inserts", no_argument, &column_inserts, 1},
{"binary-upgrade", no_argument, &binary_upgrade, 1},
{"column-inserts", no_argument, &column_inserts, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
@@ -577,6 +579,9 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
@@ -809,6 +814,7 @@ main(int argc, char **argv)
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
@@ -887,6 +893,7 @@ help(const char *progname)
printf(_(" -x, --no-privileges do not dump privileges (grant/revoke)\n"));
printf(_(" --binary-upgrade for use by upgrade utilities only\n"));
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
@@ -2290,7 +2297,8 @@ dumpDatabase(Archive *fout)
}
- appendPQExpBuffer(delQry, "DROP DATABASE %s;\n",
+ appendPQExpBuffer(delQry, "DROP DATABASE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(datname));
dbDumpId = createDumpId();
@@ -7894,7 +7902,9 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
qnspname = pg_strdup(fmtId(nspinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname);
+ appendPQExpBuffer(delq, "DROP SCHEMA %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ qnspname);
appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname);
@@ -7953,7 +7963,9 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
qextname = pg_strdup(fmtId(extinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP EXTENSION %s;\n", qextname);
+ appendPQExpBuffer(delq, "DROP EXTENSION %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ qextname);
if (!binary_upgrade)
{
@@ -8127,7 +8139,8 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
* CASCADE shouldn't be required here as for normal types since the I/O
* functions are generic and do not get dropped.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -8257,7 +8270,8 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
* CASCADE shouldn't be required here as for normal types since the I/O
* functions are generic and do not get dropped.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -8596,7 +8610,8 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
* the type and its I/O functions makes it impossible to drop the type any
* other way.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s CASCADE;\n",
qtypname);
@@ -8855,7 +8870,8 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP DOMAIN %s.",
+ appendPQExpBuffer(delq, "DROP DOMAIN %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -9069,7 +9085,8 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -9379,7 +9396,8 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
else
lanschema = NULL;
- appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s;\n",
+ appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qlanname);
if (useParams)
@@ -9923,7 +9941,8 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP FUNCTION %s.%s;\n",
+ appendPQExpBuffer(delqry, "DROP FUNCTION %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(finfo->dobj.namespace->dobj.name),
funcsig);
@@ -10141,7 +10160,8 @@ dumpCast(Archive *fout, CastInfo *cast)
delqry = createPQExpBuffer();
labelq = createPQExpBuffer();
- appendPQExpBuffer(delqry, "DROP CAST (%s AS %s);\n",
+ appendPQExpBuffer(delqry, "DROP CAST %s(%s AS %s);\n",
+ if_exists ? "IF EXISTS " : "",
getFormattedTypeName(fout, cast->castsource, zeroAsNone),
getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
@@ -10411,7 +10431,8 @@ dumpOpr(Archive *fout, OprInfo *oprinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR %s.%s;\n",
+ appendPQExpBuffer(delq, "DROP OPERATOR %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(oprinfo->dobj.namespace->dobj.name),
oprid->data);
@@ -10696,7 +10717,8 @@ dumpOpclass(Archive *fout, OpclassInfo *opcinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s",
+ appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(opcinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s",
fmtId(opcinfo->dobj.name));
@@ -11140,7 +11162,8 @@ dumpOpfamily(Archive *fout, OpfamilyInfo *opfinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR FAMILY %s",
+ appendPQExpBuffer(delq, "DROP OPERATOR FAMILY %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(opfinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s",
fmtId(opfinfo->dobj.name));
@@ -11314,7 +11337,8 @@ dumpCollation(Archive *fout, CollInfo *collinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP COLLATION %s",
+ appendPQExpBuffer(delq, "DROP COLLATION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(collinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(collinfo->dobj.name));
@@ -11411,7 +11435,8 @@ dumpConversion(Archive *fout, ConvInfo *convinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP CONVERSION %s",
+ appendPQExpBuffer(delq, "DROP CONVERSION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(convinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(convinfo->dobj.name));
@@ -11710,7 +11735,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP AGGREGATE %s.%s;\n",
+ appendPQExpBuffer(delq, "DROP AGGREGATE %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(agginfo->aggfn.dobj.namespace->dobj.name),
aggsig);
@@ -11809,7 +11835,8 @@ dumpTSParser(Archive *fout, TSParserInfo *prsinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH PARSER %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH PARSER %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(prsinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(prsinfo->dobj.name));
@@ -11896,7 +11923,8 @@ dumpTSDictionary(Archive *fout, TSDictInfo *dictinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH DICTIONARY %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH DICTIONARY %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(dictinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(dictinfo->dobj.name));
@@ -11962,7 +11990,8 @@ dumpTSTemplate(Archive *fout, TSTemplateInfo *tmplinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH TEMPLATE %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH TEMPLATE %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(tmplinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(tmplinfo->dobj.name));
@@ -12090,7 +12119,8 @@ dumpTSConfig(Archive *fout, TSConfigInfo *cfginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH CONFIGURATION %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH CONFIGURATION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(cfginfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(cfginfo->dobj.name));
@@ -12166,7 +12196,8 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
appendPQExpBufferStr(q, ";\n");
- appendPQExpBuffer(delq, "DROP FOREIGN DATA WRAPPER %s;\n",
+ appendPQExpBuffer(delq, "DROP FOREIGN DATA WRAPPER %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qfdwname);
appendPQExpBuffer(labelq, "FOREIGN DATA WRAPPER %s",
@@ -12259,7 +12290,8 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
appendPQExpBufferStr(q, ";\n");
- appendPQExpBuffer(delq, "DROP SERVER %s;\n",
+ appendPQExpBuffer(delq, "DROP SERVER %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qsrvname);
appendPQExpBuffer(labelq, "SERVER %s", qsrvname);
@@ -12377,7 +12409,9 @@ dumpUserMappings(Archive *fout,
appendPQExpBufferStr(q, ";\n");
resetPQExpBuffer(delq);
- appendPQExpBuffer(delq, "DROP USER MAPPING FOR %s", fmtId(usename));
+ appendPQExpBuffer(delq, "DROP USER MAPPING %sFOR %s",
+ if_exists ? "IF EXISTS " : "",
+ fmtId(usename));
appendPQExpBuffer(delq, " SERVER %s;\n", fmtId(servername));
resetPQExpBuffer(tag);
@@ -12984,7 +13018,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP VIEW %s.",
+ appendPQExpBuffer(delq, "DROP VIEW %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -13061,7 +13096,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP %s %s.", reltypename,
+ appendPQExpBuffer(delq, "DROP %s %s%s.", reltypename,
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -13614,7 +13650,8 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -13730,7 +13767,8 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP INDEX %s.",
+ appendPQExpBuffer(delq, "DROP INDEX %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(indxinfo->dobj.name));
@@ -13851,7 +13889,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE ONLY %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %sONLY %s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -13884,7 +13923,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE ONLY %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %sONLY %s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -14153,7 +14193,8 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP SEQUENCE %s.",
+ appendPQExpBuffer(delqry, "DROP SEQUENCE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delqry, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -14341,7 +14382,8 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP TRIGGER %s ",
+ appendPQExpBuffer(delqry, "DROP TRIGGER %s%s ",
+ if_exists ? "IF EXISTS " : "",
fmtId(tginfo->dobj.name));
appendPQExpBuffer(delqry, "ON %s.",
fmtId(tbinfo->dobj.namespace->dobj.name));
@@ -14681,7 +14723,8 @@ dumpRule(Archive *fout, RuleInfo *rinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delcmd, "DROP RULE %s ",
+ appendPQExpBuffer(delcmd, "DROP RULE %s%s ",
+ if_exists ? "IF EXISTS " : "",
fmtId(rinfo->dobj.name));
appendPQExpBuffer(delcmd, "ON %s.",
fmtId(tbinfo->dobj.namespace->dobj.name));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 05c9646..b5bc693 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -71,6 +71,7 @@ static bool verbose = false;
static int binary_upgrade = 0;
static int column_inserts = 0;
+static int if_exists = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
static int inserts = 0;
@@ -117,6 +118,7 @@ main(int argc, char *argv[])
{"attribute-inserts", no_argument, &column_inserts, 1},
{"binary-upgrade", no_argument, &binary_upgrade, 1},
{"column-inserts", no_argument, &column_inserts, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"inserts", no_argument, &inserts, 1},
@@ -348,6 +350,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --binary-upgrade");
if (column_inserts)
appendPQExpBufferStr(pgdumpopts, " --column-inserts");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (disable_dollar_quoting)
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
@@ -562,6 +566,7 @@ help(void)
printf(_(" -x, --no-privileges do not dump privileges (grant/revoke)\n"));
printf(_(" --binary-upgrade for use by upgrade utilities only\n"));
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
+ printf(_(" --if-exists don't report error if cleaned object doesn't exist\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
Hello
next version
pg_restore knows --if-exists option now
Regards
Pavel Stehule
2013/12/13 Pavel Stehule <pavel.stehule@gmail.com>
Show quoted text
Hello
I am sending a rebased patch.
Now dump generated with --if-exists option is readable by pg_restore
Regards
Pavel
Attachments:
dump-restore-if-exists-opt-2013-12-20-1.patchtext/x-patch; charset=US-ASCII; name=dump-restore-if-exists-opt-2013-12-20-1.patchDownload
commit 7c79aa77ccf53252bac8ce2302a7a0f7a1942e9b
Author: Pavel Stehule <pavel.stehule@gooddata.com>
Date: Fri Dec 20 20:13:00 2013 +0100
inital
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 8d45f24..18f7346 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -650,6 +650,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It use conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 5c6a101..670539e 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -301,6 +301,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It use conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--inserts</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index 717da42..dd8e990 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -490,6 +490,16 @@
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It use conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-data-for-failed-tables</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 6927968..83f7216 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -113,6 +113,7 @@ typedef struct _restoreOptions
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 63a8009..327ff03 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -55,7 +55,8 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt,
const int compression, ArchiveMode mode, SetupWorkerPtr setupWorkerPtr);
static void _getObjectDescription(PQExpBuffer buf, TocEntry *te,
ArchiveHandle *AH);
-static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass);
+static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt,
+ bool isData, bool acl_pass);
static char *replace_line_endings(const char *str);
static void _doSetFixedOutputState(ArchiveHandle *AH);
static void _doSetSessionAuth(ArchiveHandle *AH, const char *user);
@@ -234,6 +235,7 @@ RestoreArchive(Archive *AHX)
bool parallel_mode;
TocEntry *te;
OutputContext sav;
+
AH->stage = STAGE_INITIALIZING;
@@ -409,8 +411,24 @@ RestoreArchive(Archive *AHX)
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
- /* Drop it */
- ahprintf(AH, "%s", te->dropStmt);
+
+ if (*te->dropStmt != '\0')
+ {
+ if (ropt->if_exists)
+ {
+ int desc_len = strlen(te->desc);
+
+ /* Clause IF EXISTS can be used yet (by pg_dump) */
+ if (strncmp(te->dropStmt + desc_len + 5, " IF EXISTS", 9) != 0)
+ ahprintf(AH, "DROP %s IF EXISTS %s",
+ te->desc,
+ te->dropStmt + 6 + desc_len);
+ else
+ ahprintf(AH, "%s", te->dropStmt);
+ }
+ else
+ ahprintf(AH, "%s", te->dropStmt);
+ }
}
}
@@ -2938,9 +2956,35 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
- /* Chop "DROP " off the front and make a modifiable copy */
- char *first = pg_strdup(te->dropStmt + 5);
- char *last;
+ char *first;
+ char *last;
+
+ /* try to search IF EXISTS in DROP command */
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
+ {
+ char buffer[40];
+ size_t l;
+
+ /* IF EXISTS clause should be optional, check it*/
+ snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", type);
+ l = strlen(buffer);
+
+ if (strncmp(te->dropStmt, buffer, l) == 0)
+ {
+ /* append command type to target type */
+ appendPQExpBufferStr(buf, type);
+
+ /* skip first n chars, and create a modifieble copy */
+ first = pg_strdup(te->dropStmt + l);
+ }
+ else
+ /* DROP IF EXISTS pattern is not appliable on dropStmt */
+ first = pg_strdup(te->dropStmt + 5);
+ }
+
+ else
+ /* IF EXISTS clause was not used, simple solution */
+ first = pg_strdup(te->dropStmt + 5);
/* point to last character in string */
last = first + strlen(first) - 1;
@@ -2961,7 +3005,8 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
}
static void
-_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass)
+_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData,
+ bool acl_pass)
{
/* ACLs are dumped only during acl pass */
if (acl_pass)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 837e784..200ef9d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -140,6 +140,7 @@ static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
static int serializable_deferrable = 0;
+static int if_exists = 0;
static void help(const char *progname);
@@ -349,6 +350,7 @@ main(int argc, char **argv)
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &dump_inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
@@ -577,6 +579,9 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
@@ -809,6 +814,7 @@ main(int argc, char **argv)
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
@@ -890,6 +896,7 @@ help(const char *progname)
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
@@ -2290,7 +2297,8 @@ dumpDatabase(Archive *fout)
}
- appendPQExpBuffer(delQry, "DROP DATABASE %s;\n",
+ appendPQExpBuffer(delQry, "DROP DATABASE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(datname));
dbDumpId = createDumpId();
@@ -7894,7 +7902,9 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
qnspname = pg_strdup(fmtId(nspinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname);
+ appendPQExpBuffer(delq, "DROP SCHEMA %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ qnspname);
appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname);
@@ -7953,7 +7963,9 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
qextname = pg_strdup(fmtId(extinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP EXTENSION %s;\n", qextname);
+ appendPQExpBuffer(delq, "DROP EXTENSION %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ qextname);
if (!binary_upgrade)
{
@@ -8127,7 +8139,8 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
* CASCADE shouldn't be required here as for normal types since the I/O
* functions are generic and do not get dropped.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -8257,7 +8270,8 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
* CASCADE shouldn't be required here as for normal types since the I/O
* functions are generic and do not get dropped.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -8596,7 +8610,8 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
* the type and its I/O functions makes it impossible to drop the type any
* other way.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s CASCADE;\n",
qtypname);
@@ -8855,7 +8870,8 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP DOMAIN %s.",
+ appendPQExpBuffer(delq, "DROP DOMAIN %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -9069,7 +9085,8 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -9379,7 +9396,8 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
else
lanschema = NULL;
- appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s;\n",
+ appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qlanname);
if (useParams)
@@ -9923,7 +9941,8 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP FUNCTION %s.%s;\n",
+ appendPQExpBuffer(delqry, "DROP FUNCTION %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(finfo->dobj.namespace->dobj.name),
funcsig);
@@ -10141,7 +10160,8 @@ dumpCast(Archive *fout, CastInfo *cast)
delqry = createPQExpBuffer();
labelq = createPQExpBuffer();
- appendPQExpBuffer(delqry, "DROP CAST (%s AS %s);\n",
+ appendPQExpBuffer(delqry, "DROP CAST %s(%s AS %s);\n",
+ if_exists ? "IF EXISTS " : "",
getFormattedTypeName(fout, cast->castsource, zeroAsNone),
getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
@@ -10411,7 +10431,8 @@ dumpOpr(Archive *fout, OprInfo *oprinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR %s.%s;\n",
+ appendPQExpBuffer(delq, "DROP OPERATOR %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(oprinfo->dobj.namespace->dobj.name),
oprid->data);
@@ -10696,7 +10717,8 @@ dumpOpclass(Archive *fout, OpclassInfo *opcinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s",
+ appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(opcinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s",
fmtId(opcinfo->dobj.name));
@@ -11140,7 +11162,8 @@ dumpOpfamily(Archive *fout, OpfamilyInfo *opfinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR FAMILY %s",
+ appendPQExpBuffer(delq, "DROP OPERATOR FAMILY %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(opfinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s",
fmtId(opfinfo->dobj.name));
@@ -11314,7 +11337,8 @@ dumpCollation(Archive *fout, CollInfo *collinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP COLLATION %s",
+ appendPQExpBuffer(delq, "DROP COLLATION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(collinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(collinfo->dobj.name));
@@ -11411,7 +11435,8 @@ dumpConversion(Archive *fout, ConvInfo *convinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP CONVERSION %s",
+ appendPQExpBuffer(delq, "DROP CONVERSION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(convinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(convinfo->dobj.name));
@@ -11710,7 +11735,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP AGGREGATE %s.%s;\n",
+ appendPQExpBuffer(delq, "DROP AGGREGATE %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(agginfo->aggfn.dobj.namespace->dobj.name),
aggsig);
@@ -11809,7 +11835,8 @@ dumpTSParser(Archive *fout, TSParserInfo *prsinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH PARSER %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH PARSER %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(prsinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(prsinfo->dobj.name));
@@ -11896,7 +11923,8 @@ dumpTSDictionary(Archive *fout, TSDictInfo *dictinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH DICTIONARY %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH DICTIONARY %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(dictinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(dictinfo->dobj.name));
@@ -11962,7 +11990,8 @@ dumpTSTemplate(Archive *fout, TSTemplateInfo *tmplinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH TEMPLATE %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH TEMPLATE %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(tmplinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(tmplinfo->dobj.name));
@@ -12090,7 +12119,8 @@ dumpTSConfig(Archive *fout, TSConfigInfo *cfginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH CONFIGURATION %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH CONFIGURATION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(cfginfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(cfginfo->dobj.name));
@@ -12166,7 +12196,8 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
appendPQExpBufferStr(q, ";\n");
- appendPQExpBuffer(delq, "DROP FOREIGN DATA WRAPPER %s;\n",
+ appendPQExpBuffer(delq, "DROP FOREIGN DATA WRAPPER %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qfdwname);
appendPQExpBuffer(labelq, "FOREIGN DATA WRAPPER %s",
@@ -12259,7 +12290,8 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
appendPQExpBufferStr(q, ";\n");
- appendPQExpBuffer(delq, "DROP SERVER %s;\n",
+ appendPQExpBuffer(delq, "DROP SERVER %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qsrvname);
appendPQExpBuffer(labelq, "SERVER %s", qsrvname);
@@ -12377,7 +12409,9 @@ dumpUserMappings(Archive *fout,
appendPQExpBufferStr(q, ";\n");
resetPQExpBuffer(delq);
- appendPQExpBuffer(delq, "DROP USER MAPPING FOR %s", fmtId(usename));
+ appendPQExpBuffer(delq, "DROP USER MAPPING %sFOR %s",
+ if_exists ? "IF EXISTS " : "",
+ fmtId(usename));
appendPQExpBuffer(delq, " SERVER %s;\n", fmtId(servername));
resetPQExpBuffer(tag);
@@ -12984,7 +13018,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP VIEW %s.",
+ appendPQExpBuffer(delq, "DROP VIEW %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -13061,7 +13096,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP %s %s.", reltypename,
+ appendPQExpBuffer(delq, "DROP %s %s%s.", reltypename,
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -13614,7 +13650,8 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -13730,7 +13767,8 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP INDEX %s.",
+ appendPQExpBuffer(delq, "DROP INDEX %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(indxinfo->dobj.name));
@@ -13851,7 +13889,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE ONLY %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %sONLY %s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -13884,7 +13923,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE ONLY %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %sONLY %s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -14153,7 +14193,8 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP SEQUENCE %s.",
+ appendPQExpBuffer(delqry, "DROP SEQUENCE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delqry, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -14341,7 +14382,8 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP TRIGGER %s ",
+ appendPQExpBuffer(delqry, "DROP TRIGGER %s%s ",
+ if_exists ? "IF EXISTS " : "",
fmtId(tginfo->dobj.name));
appendPQExpBuffer(delqry, "ON %s.",
fmtId(tbinfo->dobj.namespace->dobj.name));
@@ -14681,7 +14723,8 @@ dumpRule(Archive *fout, RuleInfo *rinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delcmd, "DROP RULE %s ",
+ appendPQExpBuffer(delcmd, "DROP RULE %s%s ",
+ if_exists ? "IF EXISTS " : "",
fmtId(rinfo->dobj.name));
appendPQExpBuffer(delcmd, "ON %s.",
fmtId(tbinfo->dobj.namespace->dobj.name));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 05c9646..b5bc693 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -71,6 +71,7 @@ static bool verbose = false;
static int binary_upgrade = 0;
static int column_inserts = 0;
+static int if_exists = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
static int inserts = 0;
@@ -117,6 +118,7 @@ main(int argc, char *argv[])
{"attribute-inserts", no_argument, &column_inserts, 1},
{"binary-upgrade", no_argument, &binary_upgrade, 1},
{"column-inserts", no_argument, &column_inserts, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"inserts", no_argument, &inserts, 1},
@@ -348,6 +350,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --binary-upgrade");
if (column_inserts)
appendPQExpBufferStr(pgdumpopts, " --column-inserts");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (disable_dollar_quoting)
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
@@ -562,6 +566,7 @@ help(void)
printf(_(" -x, --no-privileges do not dump privileges (grant/revoke)\n"));
printf(_(" --binary-upgrade for use by upgrade utilities only\n"));
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
+ printf(_(" --if-exists don't report error if cleaned object doesn't exist\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 2648003..74f9c2a 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -76,6 +76,7 @@ main(int argc, char **argv)
Archive *AH;
char *inputFileSpec;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int no_data_for_failed_tables = 0;
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
@@ -116,6 +117,7 @@ main(int argc, char **argv)
* the following options don't have an equivalent short option letter
*/
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
@@ -342,6 +344,14 @@ main(int argc, char **argv)
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ if (if_exists && !opts->dropSchema)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+ opts->if_exists = if_exists;
+
if (opts->formatName)
{
switch (opts->formatName[0])
@@ -456,6 +466,7 @@ usage(const char *progname)
printf(_(" -x, --no-privileges skip restoration of access privileges (grant/revoke)\n"));
printf(_(" -1, --single-transaction restore as a single transaction\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
Hi Pavel,
I have reviewed the patch and here are my concerns and notes:
POSITIVES:
---
1. Patch applies with some white-space errors.
2. make / make install / make check is smooth. No issues as such.
3. Feature looks good as well.
4. NO concern on overall design.
5. Good work.
NEGATIVES:
---
Here are the points which I see in the review and would like you to have
your attention.
1.
+ It use conditional commands (with <literal>IF EXISTS</literal>
Grammar mistakes. use => uses
2.
@@ -55,7 +55,8 @@ static ArchiveHandle *_allocAH(const char *FileSpec,
const ArchiveFormat fmt,
const int compression, ArchiveMode mode, SetupWorkerPtr
setupWorkerPtr);
static void _getObjectDescription(PQExpBuffer buf, TocEntry *te,
ArchiveHandle *AH);
-static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions
*ropt, bool isData, bool acl_pass);
+static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions
*ropt,
+ bool isData, bool acl_pass);
static char *replace_line_endings(const char *str);
static void _doSetFixedOutputState(ArchiveHandle *AH);
static void _doSetSessionAuth(ArchiveHandle *AH, const char *user);
@@ -234,6 +235,7 @@ RestoreArchive(Archive *AHX)
bool parallel_mode;
TocEntry *te;
OutputContext sav;
+
AH->stage = STAGE_INITIALIZING;
@@ -2961,7 +3005,8 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te,
ArchiveHandle *AH)
}
static void
-_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool
isData, bool acl_pass)
+_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool
isData,
+ bool acl_pass)
{
/* ACLs are dumped only during acl pass */
if (acl_pass)
Above changes are NOT at all related to the patch. Please remove them even
though they are clean-up like changes. Don't mix them with actual changes.
3.
+ if (strncmp(te->dropStmt + desc_len + 5, " IF
EXISTS", 9) != 0)
" IF EXISTS" has 10 characters NOT 9.
4.
+ if (strncmp(te->dropStmt + desc_len + 5, " IF
EXISTS", 9) != 0)
+ ahprintf(AH, "DROP %s IF EXISTS %s",
+ te->desc,
+ te->dropStmt + 6 + desc_len);
Here you have used strncmp, starting at te->dropStmt + X,
where X = desc_len + 5. While adding back you used X = 6 + desc_len.
First time you used 5 as you added space in comparison but for adding back
we
want past space location and thus you have used 6. That's correct, but
little
bit confusing. Why not you simply used
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
to check whether drop statement has "IF EXISTS" or not like you did at some
other place. This will remove my concern 3 and above confusion as well.
What you think ?
5.
+ }
+
+ else
Extra line before else part. Please remove it for consistency.
6.
+ printf(_(" --if-exists use IF EXISTS when dropping
objects\n")); (pg_dump)
+ printf(_(" --if-exists don't report error if cleaned
object doesn't exist\n")); (pg_dumpall)
+ printf(_(" --if-exists use IF EXISTS when dropping
objects\n")); (pg_restore)
Please have same message for all three.
7.
printf(_(" --binary-upgrade for use by upgrade utilities
only\n"));
printf(_(" --column-inserts dump data as INSERT commands
with column names\n"));
+ printf(_(" --if-exists don't report error if cleaned
object doesn't exist\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use
SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during
data-only restore\n"));
Please maintain order like pg_dump and pg_restore. Also at variable
declaration
and at options parsing mechanism.
8.
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean
option\n");
Are we really want to exit when -c is not provided ? Can't we simply ignore
--if-exists in that case (or with emitting a warning) ?
Marking "Waiting on author".
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Hello
2014/1/16 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Hi Pavel,
I have reviewed the patch and here are my concerns and notes:
POSITIVES:
---
1. Patch applies with some white-space errors.
2. make / make install / make check is smooth. No issues as such.
3. Feature looks good as well.
4. NO concern on overall design.
5. Good work.NEGATIVES:
---Here are the points which I see in the review and would like you to have
your attention.1.
+ It use conditional commands (with <literal>IF EXISTS</literal>Grammar mistakes. use => uses
2. @@ -55,7 +55,8 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt, const int compression, ArchiveMode mode, SetupWorkerPtr setupWorkerPtr); static void _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH); -static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass); +static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, + bool isData, bool acl_pass); static char *replace_line_endings(const char *str); static void _doSetFixedOutputState(ArchiveHandle *AH); static void _doSetSessionAuth(ArchiveHandle *AH, const char *user); @@ -234,6 +235,7 @@ RestoreArchive(Archive *AHX) bool parallel_mode; TocEntry *te; OutputContext sav; +AH->stage = STAGE_INITIALIZING;
@@ -2961,7 +3005,8 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te,
ArchiveHandle *AH)
}static void -_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass) +_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, + bool acl_pass) { /* ACLs are dumped only during acl pass */ if (acl_pass)Above changes are NOT at all related to the patch. Please remove them even
though they are clean-up like changes. Don't mix them with actual changes.3.
+ if (strncmp(te->dropStmt + desc_len + 5, " IF
EXISTS", 9) != 0)" IF EXISTS" has 10 characters NOT 9.
4. + if (strncmp(te->dropStmt + desc_len + 5, " IF EXISTS", 9) != 0) + ahprintf(AH, "DROP %s IF EXISTS %s", + te->desc, + te->dropStmt + 6 + desc_len);Here you have used strncmp, starting at te->dropStmt + X,
where X = desc_len + 5. While adding back you used X = 6 + desc_len.
First time you used 5 as you added space in comparison but for adding back
we
want past space location and thus you have used 6. That's correct, but
little
bit confusing. Why not you simply used
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
to check whether drop statement has "IF EXISTS" or not like you did at some
other place. This will remove my concern 3 and above confusion as well.
What you think ?5. + } + + elseExtra line before else part. Please remove it for consistency.
6. + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_dump) + printf(_(" --if-exists don't report error if cleaned object doesn't exist\n")); (pg_dumpall) + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_restore)Please have same message for all three.
7.
printf(_(" --binary-upgrade for use by upgrade utilities
only\n"));
printf(_(" --column-inserts dump data as INSERT commands
with column names\n"));
+ printf(_(" --if-exists don't report error if cleaned
object doesn't exist\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use
SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during
data-only restore\n"));Please maintain order like pg_dump and pg_restore. Also at variable
declaration
and at options parsing mechanism.
I fixed a previous issue, see a attachment please
8. + if (if_exists && !outputClean) + exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");Are we really want to exit when -c is not provided ? Can't we simply ignore
--if-exists in that case (or with emitting a warning) ?
This behave is based on a talk related to proposal of this feature - and I
am thinking, this behave is little bit safer - ignoring requested
functionality is not what I like. And a error message is simple and clean
in this case - is not difficult to use it and it is not difficult to fix
missing option for user
Regards
Pavel
Show quoted text
Marking "Waiting on author".
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Attachments:
dump-restore-if-exists-opt-2014-01-17-1.patchtext/x-patch; charset=US-ASCII; name=dump-restore-if-exists-opt-2014-01-17-1.patchDownload
commit 3fecb7805261e96db764fffcae2bb912538903f5
Author: Pavel Stehule <pavel.stehule@gmail.com>
Date: Fri Jan 17 13:37:51 2014 +0100
review related fixes
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 8d45f24..c39767b 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -650,6 +650,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 5c6a101..ba6583d 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -301,6 +301,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--inserts</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index 717da42..55326d5 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -490,6 +490,16 @@
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-data-for-failed-tables</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 6927968..83f7216 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -113,6 +113,7 @@ typedef struct _restoreOptions
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 7fc0288..0677712 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -413,8 +413,30 @@ RestoreArchive(Archive *AHX)
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
- /* Drop it */
- ahprintf(AH, "%s", te->dropStmt);
+
+ if (*te->dropStmt != '\0')
+ {
+ /* Inject IF EXISTS clause when it is required. */
+ if (ropt->if_exists)
+ {
+ char buffer[40];
+ size_t l;
+
+ /* But do it only, when it is not there yet. */
+ snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS",
+ te->desc);
+ l = strlen(buffer);
+
+ if (strncmp(te->dropStmt, buffer, l) != 0)
+ {
+ ahprintf(AH, "DROP %s IF EXISTS %s",
+ te->desc,
+ te->dropStmt + l);
+ }
+ else
+ ahprintf(AH, "%s", te->dropStmt);
+ }
+ }
}
}
@@ -2942,9 +2964,39 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
- /* Chop "DROP " off the front and make a modifiable copy */
- char *first = pg_strdup(te->dropStmt + 5);
- char *last;
+ char *first;
+ char *last;
+
+ /*
+ * Object description is based on dropStmt statement. But
+ * a drop statements can be enhanced about IF EXISTS clause.
+ * We have to increase a offset in this case, "IF EXISTS"
+ * should not be included on object description.
+ */
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
+ {
+ char buffer[40];
+ size_t l;
+
+ /* IF EXISTS clause should be optional, check it*/
+ snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", type);
+ l = strlen(buffer);
+
+ if (strncmp(te->dropStmt, buffer, l) == 0)
+ {
+ /* append command type to target type */
+ appendPQExpBufferStr(buf, type);
+
+ /* skip first n chars, and create a modifieble copy */
+ first = pg_strdup(te->dropStmt + l);
+ }
+ else
+ /* DROP IF EXISTS pattern is not appliable on dropStmt */
+ first = pg_strdup(te->dropStmt + 5);
+ }
+ else
+ /* IF EXISTS clause was not used, simple solution */
+ first = pg_strdup(te->dropStmt + 5);
/* point to last character in string */
last = first + strlen(first) - 1;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 3862f05..1227d55 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -136,6 +136,7 @@ static int binary_upgrade = 0;
static int disable_dollar_quoting = 0;
static int dump_inserts = 0;
static int column_inserts = 0;
+static int if_exists = 0;
static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
@@ -349,6 +350,7 @@ main(int argc, char **argv)
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &dump_inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
@@ -577,6 +579,9 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
@@ -809,6 +814,7 @@ main(int argc, char **argv)
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
@@ -890,6 +896,7 @@ help(const char *progname)
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
@@ -2290,7 +2297,8 @@ dumpDatabase(Archive *fout)
}
- appendPQExpBuffer(delQry, "DROP DATABASE %s;\n",
+ appendPQExpBuffer(delQry, "DROP DATABASE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(datname));
dbDumpId = createDumpId();
@@ -7894,7 +7902,9 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
qnspname = pg_strdup(fmtId(nspinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname);
+ appendPQExpBuffer(delq, "DROP SCHEMA %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ qnspname);
appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname);
@@ -7953,7 +7963,9 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
qextname = pg_strdup(fmtId(extinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP EXTENSION %s;\n", qextname);
+ appendPQExpBuffer(delq, "DROP EXTENSION %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ qextname);
if (!binary_upgrade)
{
@@ -8127,7 +8139,8 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
* CASCADE shouldn't be required here as for normal types since the I/O
* functions are generic and do not get dropped.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -8257,7 +8270,8 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
* CASCADE shouldn't be required here as for normal types since the I/O
* functions are generic and do not get dropped.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -8596,7 +8610,8 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
* the type and its I/O functions makes it impossible to drop the type any
* other way.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s CASCADE;\n",
qtypname);
@@ -8855,7 +8870,8 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP DOMAIN %s.",
+ appendPQExpBuffer(delq, "DROP DOMAIN %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -9069,7 +9085,8 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -9379,7 +9396,8 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
else
lanschema = NULL;
- appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s;\n",
+ appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qlanname);
if (useParams)
@@ -9923,7 +9941,8 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP FUNCTION %s.%s;\n",
+ appendPQExpBuffer(delqry, "DROP FUNCTION %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(finfo->dobj.namespace->dobj.name),
funcsig);
@@ -10141,7 +10160,8 @@ dumpCast(Archive *fout, CastInfo *cast)
delqry = createPQExpBuffer();
labelq = createPQExpBuffer();
- appendPQExpBuffer(delqry, "DROP CAST (%s AS %s);\n",
+ appendPQExpBuffer(delqry, "DROP CAST %s(%s AS %s);\n",
+ if_exists ? "IF EXISTS " : "",
getFormattedTypeName(fout, cast->castsource, zeroAsNone),
getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
@@ -10411,7 +10431,8 @@ dumpOpr(Archive *fout, OprInfo *oprinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR %s.%s;\n",
+ appendPQExpBuffer(delq, "DROP OPERATOR %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(oprinfo->dobj.namespace->dobj.name),
oprid->data);
@@ -10696,7 +10717,8 @@ dumpOpclass(Archive *fout, OpclassInfo *opcinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s",
+ appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(opcinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s",
fmtId(opcinfo->dobj.name));
@@ -11140,7 +11162,8 @@ dumpOpfamily(Archive *fout, OpfamilyInfo *opfinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR FAMILY %s",
+ appendPQExpBuffer(delq, "DROP OPERATOR FAMILY %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(opfinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s",
fmtId(opfinfo->dobj.name));
@@ -11314,7 +11337,8 @@ dumpCollation(Archive *fout, CollInfo *collinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP COLLATION %s",
+ appendPQExpBuffer(delq, "DROP COLLATION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(collinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(collinfo->dobj.name));
@@ -11411,7 +11435,8 @@ dumpConversion(Archive *fout, ConvInfo *convinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP CONVERSION %s",
+ appendPQExpBuffer(delq, "DROP CONVERSION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(convinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(convinfo->dobj.name));
@@ -11723,7 +11748,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP AGGREGATE %s.%s;\n",
+ appendPQExpBuffer(delq, "DROP AGGREGATE %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(agginfo->aggfn.dobj.namespace->dobj.name),
aggsig);
@@ -11822,7 +11848,8 @@ dumpTSParser(Archive *fout, TSParserInfo *prsinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH PARSER %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH PARSER %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(prsinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(prsinfo->dobj.name));
@@ -11909,7 +11936,8 @@ dumpTSDictionary(Archive *fout, TSDictInfo *dictinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH DICTIONARY %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH DICTIONARY %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(dictinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(dictinfo->dobj.name));
@@ -11975,7 +12003,8 @@ dumpTSTemplate(Archive *fout, TSTemplateInfo *tmplinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH TEMPLATE %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH TEMPLATE %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(tmplinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(tmplinfo->dobj.name));
@@ -12103,7 +12132,8 @@ dumpTSConfig(Archive *fout, TSConfigInfo *cfginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH CONFIGURATION %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH CONFIGURATION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(cfginfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(cfginfo->dobj.name));
@@ -12179,7 +12209,8 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
appendPQExpBufferStr(q, ";\n");
- appendPQExpBuffer(delq, "DROP FOREIGN DATA WRAPPER %s;\n",
+ appendPQExpBuffer(delq, "DROP FOREIGN DATA WRAPPER %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qfdwname);
appendPQExpBuffer(labelq, "FOREIGN DATA WRAPPER %s",
@@ -12272,7 +12303,8 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
appendPQExpBufferStr(q, ";\n");
- appendPQExpBuffer(delq, "DROP SERVER %s;\n",
+ appendPQExpBuffer(delq, "DROP SERVER %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qsrvname);
appendPQExpBuffer(labelq, "SERVER %s", qsrvname);
@@ -12390,7 +12422,9 @@ dumpUserMappings(Archive *fout,
appendPQExpBufferStr(q, ";\n");
resetPQExpBuffer(delq);
- appendPQExpBuffer(delq, "DROP USER MAPPING FOR %s", fmtId(usename));
+ appendPQExpBuffer(delq, "DROP USER MAPPING %sFOR %s",
+ if_exists ? "IF EXISTS " : "",
+ fmtId(usename));
appendPQExpBuffer(delq, " SERVER %s;\n", fmtId(servername));
resetPQExpBuffer(tag);
@@ -12997,7 +13031,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP VIEW %s.",
+ appendPQExpBuffer(delq, "DROP VIEW %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -13074,7 +13109,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP %s %s.", reltypename,
+ appendPQExpBuffer(delq, "DROP %s %s%s.", reltypename,
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -13627,7 +13663,8 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -13743,7 +13780,8 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP INDEX %s.",
+ appendPQExpBuffer(delq, "DROP INDEX %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(indxinfo->dobj.name));
@@ -13864,7 +13902,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE ONLY %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %sONLY %s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -13897,7 +13936,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE ONLY %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %sONLY %s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -14166,7 +14206,8 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP SEQUENCE %s.",
+ appendPQExpBuffer(delqry, "DROP SEQUENCE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delqry, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -14362,7 +14403,8 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP TRIGGER %s ",
+ appendPQExpBuffer(delqry, "DROP TRIGGER %s%s ",
+ if_exists ? "IF EXISTS " : "",
fmtId(tginfo->dobj.name));
appendPQExpBuffer(delqry, "ON %s.",
fmtId(tbinfo->dobj.namespace->dobj.name));
@@ -14710,7 +14752,8 @@ dumpRule(Archive *fout, RuleInfo *rinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delcmd, "DROP RULE %s ",
+ appendPQExpBuffer(delcmd, "DROP RULE %s%s ",
+ if_exists ? "IF EXISTS " : "",
fmtId(rinfo->dobj.name));
appendPQExpBuffer(delcmd, "ON %s.",
fmtId(tbinfo->dobj.namespace->dobj.name));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 193c1a0..e54a829 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -73,6 +73,7 @@ static int binary_upgrade = 0;
static int column_inserts = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
+static int if_exists = 0;
static int inserts = 0;
static int no_tablespaces = 0;
static int use_setsessauth = 0;
@@ -119,6 +120,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &no_tablespaces, 1},
@@ -348,6 +350,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --binary-upgrade");
if (column_inserts)
appendPQExpBufferStr(pgdumpopts, " --column-inserts");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (disable_dollar_quoting)
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
@@ -564,6 +568,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 2648003..74f9c2a 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -76,6 +76,7 @@ main(int argc, char **argv)
Archive *AH;
char *inputFileSpec;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int no_data_for_failed_tables = 0;
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
@@ -116,6 +117,7 @@ main(int argc, char **argv)
* the following options don't have an equivalent short option letter
*/
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
@@ -342,6 +344,14 @@ main(int argc, char **argv)
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ if (if_exists && !opts->dropSchema)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+ opts->if_exists = if_exists;
+
if (opts->formatName)
{
switch (opts->formatName[0])
@@ -456,6 +466,7 @@ usage(const char *progname)
printf(_(" -x, --no-privileges skip restoration of access privileges (grant/revoke)\n"));
printf(_(" -1, --single-transaction restore as a single transaction\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
Hi Pavel,
Consider following test scenario:
mydb=# \d emp
Table "public.emp"
Column | Type | Modifiers
--------+---------+-----------
empno | integer | not null
deptno | integer |
ename | text |
Indexes:
"emp_pkey" PRIMARY KEY, btree (empno)
Foreign-key constraints:
"emp_deptno_fkey" FOREIGN KEY (deptno) REFERENCES dept(deptno)
mydb=# \d dept
Table "public.dept"
Column | Type | Modifiers
--------+---------+-----------
deptno | integer | not null
dname | text |
Indexes:
"dept_pkey" PRIMARY KEY, btree (deptno)
Referenced by:
TABLE "emp" CONSTRAINT "emp_deptno_fkey" FOREIGN KEY (deptno)
REFERENCES dept(deptno)
mydb=# \q
jeevan@ubuntu:~/pg_master$ ./install/bin/pg_dump -d mydb --if-exists -c >
mydb_ic.dmp
I see following lines in dump which looks certainly wrong:
===
DROP FK CONSTRAINT IF EXISTS ublic.emp DROP CONSTRAINT emp_deptno_fkey;
DROP CONSTRAINT IF EXISTS Y public.emp DROP CONSTRAINT emp_pkey;
DROP CONSTRAINT IF EXISTS Y public.dept DROP CONSTRAINT dept_pkey;
When try to restore, as expected it is throwing an error:
===
psql:mydb_ic.dmp:14: ERROR: syntax error at or near "FK"
LINE 1: DROP FK CONSTRAINT IF EXISTS ublic.emp DROP CONSTRAINT emp_d...
^
psql:mydb_ic.dmp:15: ERROR: syntax error at or near "CONSTRAINT"
LINE 1: DROP CONSTRAINT IF EXISTS Y public.emp DROP CONSTRAINT emp_p...
^
psql:mydb_ic.dmp:16: ERROR: syntax error at or near "CONSTRAINT"
LINE 1: DROP CONSTRAINT IF EXISTS Y public.dept DROP CONSTRAINT dept...
^
Note:
===
Commands which are in form of ALTER TABLE ... DROP are failing.
You need to test each and every object with DROP .. IF EXISTS command.
Better write small test-case with all objects included.
Following logic has flaw:
===
diff --git a/src/bin/pg_dump/pg_backup_archiver.c
b/src/bin/pg_dump/pg_backup_archiver.c
index 7fc0288..0677712 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -413,8 +413,30 @@ RestoreArchive(Archive *AHX)
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
- /* Drop it */
- ahprintf(AH, "%s", te->dropStmt);
+
+ if (*te->dropStmt != '\0')
+ {
+ /* Inject IF EXISTS clause when it is required. */
+ if (ropt->if_exists)
+ {
+ char buffer[40];
+ size_t l;
+
+ /* But do it only, when it is not there yet. */
+ snprintf(buffer, sizeof(buffer), "DROP %s IF
EXISTS",
+ te->desc);
+ l = strlen(buffer);
+
+ if (strncmp(te->dropStmt, buffer, l) != 0)
+ {
+ ahprintf(AH, "DROP %s IF EXISTS %s",
+ te->desc,
+ te->dropStmt + l);
+ }
+ else
+ ahprintf(AH, "%s", te->dropStmt);
+ }
+ }
}
}
Also:
===
1.
This is still out of sync.
@@ -348,6 +350,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --binary-upgrade");
if (column_inserts)
appendPQExpBufferStr(pgdumpopts, " --column-inserts");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (disable_dollar_quoting)
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
2.
Spell check required:
+ /* skip first n chars, and create a modifieble copy */
modifieble => modifiable
+ /* DROP IF EXISTS pattern is not appliable on dropStmt */
appliable => applicable
3.
+ /*
+ * Object description is based on dropStmt statement. But
+ * a drop statements can be enhanced about IF EXISTS clause.
+ * We have to increase a offset in this case, "IF EXISTS"
+ * should not be included on object description.
+ */
Looks like you need to re-phrase these comments line. Something like:
/*
* Object description is based on dropStmt statement which may have
* IF EXISTS clause. Thus we need to update an offset such that it
* won't be included in the object description.
*/
Or as per your choice.
Need to have careful thought on a bug mentioned above.
Thanks
On Fri, Jan 17, 2014 at 6:23 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
Hello
2014/1/16 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Hi Pavel,
I have reviewed the patch and here are my concerns and notes:
POSITIVES:
---
1. Patch applies with some white-space errors.
2. make / make install / make check is smooth. No issues as such.
3. Feature looks good as well.
4. NO concern on overall design.
5. Good work.NEGATIVES:
---Here are the points which I see in the review and would like you to have
your attention.1.
+ It use conditional commands (with <literal>IF EXISTS</literal>Grammar mistakes. use => uses
2. @@ -55,7 +55,8 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt, const int compression, ArchiveMode mode, SetupWorkerPtr setupWorkerPtr); static void _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH); -static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass); +static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, + bool isData, bool acl_pass); static char *replace_line_endings(const char *str); static void _doSetFixedOutputState(ArchiveHandle *AH); static void _doSetSessionAuth(ArchiveHandle *AH, const char *user); @@ -234,6 +235,7 @@ RestoreArchive(Archive *AHX) bool parallel_mode; TocEntry *te; OutputContext sav; +AH->stage = STAGE_INITIALIZING;
@@ -2961,7 +3005,8 @@ _getObjectDescription(PQExpBuffer buf, TocEntry
*te, ArchiveHandle *AH)
}static void -_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass) +_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, + bool acl_pass) { /* ACLs are dumped only during acl pass */ if (acl_pass)Above changes are NOT at all related to the patch. Please remove them even
though they are clean-up like changes. Don't mix them with actual changes.3.
+ if (strncmp(te->dropStmt + desc_len + 5, " IF
EXISTS", 9) != 0)" IF EXISTS" has 10 characters NOT 9.
4. + if (strncmp(te->dropStmt + desc_len + 5, " IF EXISTS", 9) != 0) + ahprintf(AH, "DROP %s IF EXISTS %s", + te->desc, + te->dropStmt + 6 + desc_len);Here you have used strncmp, starting at te->dropStmt + X,
where X = desc_len + 5. While adding back you used X = 6 + desc_len.
First time you used 5 as you added space in comparison but for adding
back we
want past space location and thus you have used 6. That's correct, but
little
bit confusing. Why not you simply used
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
to check whether drop statement has "IF EXISTS" or not like you did at
some
other place. This will remove my concern 3 and above confusion as well.
What you think ?5. + } + + elseExtra line before else part. Please remove it for consistency.
6. + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_dump) + printf(_(" --if-exists don't report error if cleaned object doesn't exist\n")); (pg_dumpall) + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_restore)Please have same message for all three.
7.
printf(_(" --binary-upgrade for use by upgrade utilities
only\n"));
printf(_(" --column-inserts dump data as INSERT commands
with column names\n"));
+ printf(_(" --if-exists don't report error if
cleaned object doesn't exist\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use
SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during
data-only restore\n"));Please maintain order like pg_dump and pg_restore. Also at variable
declaration
and at options parsing mechanism.I fixed a previous issue, see a attachment please
8. + if (if_exists && !outputClean) + exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");Are we really want to exit when -c is not provided ? Can't we simply
ignore
--if-exists in that case (or with emitting a warning) ?This behave is based on a talk related to proposal of this feature - and I
am thinking, this behave is little bit safer - ignoring requested
functionality is not what I like. And a error message is simple and clean
in this case - is not difficult to use it and it is not difficult to fix
missing option for userRegards
Pavel
Marking "Waiting on author".
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Phone: +91 20 30589500
Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb
This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.
Tomorrow I'll send updated version
Regards
Pavel
2014/1/21 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Show quoted text
Hi Pavel,
Consider following test scenario:
mydb=# \d emp
Table "public.emp"
Column | Type | Modifiers
--------+---------+-----------
empno | integer | not null
deptno | integer |
ename | text |
Indexes:
"emp_pkey" PRIMARY KEY, btree (empno)
Foreign-key constraints:
"emp_deptno_fkey" FOREIGN KEY (deptno) REFERENCES dept(deptno)mydb=# \d dept
Table "public.dept"
Column | Type | Modifiers
--------+---------+-----------
deptno | integer | not null
dname | text |
Indexes:
"dept_pkey" PRIMARY KEY, btree (deptno)
Referenced by:
TABLE "emp" CONSTRAINT "emp_deptno_fkey" FOREIGN KEY (deptno)
REFERENCES dept(deptno)mydb=# \q
jeevan@ubuntu:~/pg_master$ ./install/bin/pg_dump -d mydb --if-exists -c >
mydb_ic.dmpI see following lines in dump which looks certainly wrong:
===DROP FK CONSTRAINT IF EXISTS ublic.emp DROP CONSTRAINT emp_deptno_fkey;
DROP CONSTRAINT IF EXISTS Y public.emp DROP CONSTRAINT emp_pkey;
DROP CONSTRAINT IF EXISTS Y public.dept DROP CONSTRAINT dept_pkey;When try to restore, as expected it is throwing an error:
===psql:mydb_ic.dmp:14: ERROR: syntax error at or near "FK"
LINE 1: DROP FK CONSTRAINT IF EXISTS ublic.emp DROP CONSTRAINT emp_d...
^
psql:mydb_ic.dmp:15: ERROR: syntax error at or near "CONSTRAINT"
LINE 1: DROP CONSTRAINT IF EXISTS Y public.emp DROP CONSTRAINT emp_p...
^
psql:mydb_ic.dmp:16: ERROR: syntax error at or near "CONSTRAINT"
LINE 1: DROP CONSTRAINT IF EXISTS Y public.dept DROP CONSTRAINT dept...
^Note:
===
Commands which are in form of ALTER TABLE ... DROP are failing.
You need to test each and every object with DROP .. IF EXISTS command.
Better write small test-case with all objects included.Following logic has flaw: === diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 7fc0288..0677712 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -413,8 +413,30 @@ RestoreArchive(Archive *AHX) /* Select owner and schema as necessary */ _becomeOwner(AH, te); _selectOutputSchema(AH, te->namespace); - /* Drop it */ - ahprintf(AH, "%s", te->dropStmt); + + if (*te->dropStmt != '\0') + { + /* Inject IF EXISTS clause when it is required. */ + if (ropt->if_exists) + { + char buffer[40]; + size_t l; + + /* But do it only, when it is not there yet. */ + snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", + te->desc); + l = strlen(buffer); + + if (strncmp(te->dropStmt, buffer, l) != 0) + {+ ahprintf(AH, "DROP %s IF EXISTS %s", + te->desc, + te->dropStmt + l); + } + else + ahprintf(AH, "%s", te->dropStmt); + } + } } }Also:
===1.
This is still out of sync.@@ -348,6 +350,8 @@ main(int argc, char *argv[]) appendPQExpBufferStr(pgdumpopts, " --binary-upgrade"); if (column_inserts) appendPQExpBufferStr(pgdumpopts, " --column-inserts"); + if (if_exists) + appendPQExpBufferStr(pgdumpopts, " --if-exists"); if (disable_dollar_quoting) appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting"); if (disable_triggers)2.
Spell check required:+ /* skip first n chars, and create a modifieble copy */
modifieble => modifiable
+ /* DROP IF EXISTS pattern is not appliable on dropStmt */
appliable => applicable
3.
+ /* + * Object description is based on dropStmt statement. But + * a drop statements can be enhanced about IF EXISTS clause. + * We have to increase a offset in this case, "IF EXISTS" + * should not be included on object description. + */Looks like you need to re-phrase these comments line. Something like:
/*
* Object description is based on dropStmt statement which may have
* IF EXISTS clause. Thus we need to update an offset such that it
* won't be included in the object description.
*/
Or as per your choice.Need to have careful thought on a bug mentioned above.
Thanks
On Fri, Jan 17, 2014 at 6:23 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
Hello
2014/1/16 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Hi Pavel,
I have reviewed the patch and here are my concerns and notes:
POSITIVES:
---
1. Patch applies with some white-space errors.
2. make / make install / make check is smooth. No issues as such.
3. Feature looks good as well.
4. NO concern on overall design.
5. Good work.NEGATIVES:
---Here are the points which I see in the review and would like you to have
your attention.1.
+ It use conditional commands (with <literal>IF EXISTS</literal>Grammar mistakes. use => uses
2. @@ -55,7 +55,8 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt, const int compression, ArchiveMode mode, SetupWorkerPtr setupWorkerPtr); static void _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH); -static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass); +static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, + bool isData, bool acl_pass); static char *replace_line_endings(const char *str); static void _doSetFixedOutputState(ArchiveHandle *AH); static void _doSetSessionAuth(ArchiveHandle *AH, const char *user); @@ -234,6 +235,7 @@ RestoreArchive(Archive *AHX) bool parallel_mode; TocEntry *te; OutputContext sav; +AH->stage = STAGE_INITIALIZING;
@@ -2961,7 +3005,8 @@ _getObjectDescription(PQExpBuffer buf, TocEntry
*te, ArchiveHandle *AH)
}static void -_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass) +_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, + bool acl_pass) { /* ACLs are dumped only during acl pass */ if (acl_pass)Above changes are NOT at all related to the patch. Please remove them
even
though they are clean-up like changes. Don't mix them with actual
changes.3.
+ if (strncmp(te->dropStmt + desc_len + 5, " IF
EXISTS", 9) != 0)" IF EXISTS" has 10 characters NOT 9.
4. + if (strncmp(te->dropStmt + desc_len + 5, " IF EXISTS", 9) != 0) + ahprintf(AH, "DROP %s IF EXISTS %s", + te->desc, + te->dropStmt + 6 + desc_len);Here you have used strncmp, starting at te->dropStmt + X,
where X = desc_len + 5. While adding back you used X = 6 + desc_len.
First time you used 5 as you added space in comparison but for adding
back we
want past space location and thus you have used 6. That's correct, but
little
bit confusing. Why not you simply used
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
to check whether drop statement has "IF EXISTS" or not like you did at
some
other place. This will remove my concern 3 and above confusion as well.
What you think ?5. + } + + elseExtra line before else part. Please remove it for consistency.
6. + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_dump) + printf(_(" --if-exists don't report error if cleaned object doesn't exist\n")); (pg_dumpall) + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_restore)Please have same message for all three.
7.
printf(_(" --binary-upgrade for use by upgrade
utilities only\n"));
printf(_(" --column-inserts dump data as INSERT
commands with column names\n"));
+ printf(_(" --if-exists don't report error if
cleaned object doesn't exist\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use
SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during
data-only restore\n"));Please maintain order like pg_dump and pg_restore. Also at variable
declaration
and at options parsing mechanism.I fixed a previous issue, see a attachment please
8. + if (if_exists && !outputClean) + exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");Are we really want to exit when -c is not provided ? Can't we simply
ignore
--if-exists in that case (or with emitting a warning) ?This behave is based on a talk related to proposal of this feature - and
I am thinking, this behave is little bit safer - ignoring requested
functionality is not what I like. And a error message is simple and clean
in this case - is not difficult to use it and it is not difficult to fix
missing option for userRegards
Pavel
Marking "Waiting on author".
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL CompanyPhone: +91 20 30589500
Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedbThis e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.
Hello
I fixed all described issues. There was a more mistakes, that I fixed -
main mistake was in work with te->desc variable.
You propose a regress tests for pg_dump? I searched in mailing lists and
there was some proposals about it. I am not against, but I have to do some
research and better be this as separate patch.
This patch requires b152c6cd0de1827ba58756e24e18110cf902182a commit.
Regards
Pavel
2014-01-21 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Show quoted text
Hi Pavel,
Consider following test scenario:
mydb=# \d emp
Table "public.emp"
Column | Type | Modifiers
--------+---------+-----------
empno | integer | not null
deptno | integer |
ename | text |
Indexes:
"emp_pkey" PRIMARY KEY, btree (empno)
Foreign-key constraints:
"emp_deptno_fkey" FOREIGN KEY (deptno) REFERENCES dept(deptno)mydb=# \d dept
Table "public.dept"
Column | Type | Modifiers
--------+---------+-----------
deptno | integer | not null
dname | text |
Indexes:
"dept_pkey" PRIMARY KEY, btree (deptno)
Referenced by:
TABLE "emp" CONSTRAINT "emp_deptno_fkey" FOREIGN KEY (deptno)
REFERENCES dept(deptno)mydb=# \q
jeevan@ubuntu:~/pg_master$ ./install/bin/pg_dump -d mydb --if-exists -c >
mydb_ic.dmpI see following lines in dump which looks certainly wrong:
===DROP FK CONSTRAINT IF EXISTS ublic.emp DROP CONSTRAINT emp_deptno_fkey;
DROP CONSTRAINT IF EXISTS Y public.emp DROP CONSTRAINT emp_pkey;
DROP CONSTRAINT IF EXISTS Y public.dept DROP CONSTRAINT dept_pkey;When try to restore, as expected it is throwing an error:
===psql:mydb_ic.dmp:14: ERROR: syntax error at or near "FK"
LINE 1: DROP FK CONSTRAINT IF EXISTS ublic.emp DROP CONSTRAINT emp_d...
^
psql:mydb_ic.dmp:15: ERROR: syntax error at or near "CONSTRAINT"
LINE 1: DROP CONSTRAINT IF EXISTS Y public.emp DROP CONSTRAINT emp_p...
^
psql:mydb_ic.dmp:16: ERROR: syntax error at or near "CONSTRAINT"
LINE 1: DROP CONSTRAINT IF EXISTS Y public.dept DROP CONSTRAINT dept...
^Note:
===
Commands which are in form of ALTER TABLE ... DROP are failing.
You need to test each and every object with DROP .. IF EXISTS command.
Better write small test-case with all objects included.Following logic has flaw: === diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 7fc0288..0677712 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -413,8 +413,30 @@ RestoreArchive(Archive *AHX) /* Select owner and schema as necessary */ _becomeOwner(AH, te); _selectOutputSchema(AH, te->namespace); - /* Drop it */ - ahprintf(AH, "%s", te->dropStmt); + + if (*te->dropStmt != '\0') + { + /* Inject IF EXISTS clause when it is required. */ + if (ropt->if_exists) + { + char buffer[40]; + size_t l; + + /* But do it only, when it is not there yet. */ + snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", + te->desc); + l = strlen(buffer); + + if (strncmp(te->dropStmt, buffer, l) != 0) + {+ ahprintf(AH, "DROP %s IF EXISTS %s", + te->desc, + te->dropStmt + l); + } + else + ahprintf(AH, "%s", te->dropStmt); + } + } } }Also:
===1.
This is still out of sync.@@ -348,6 +350,8 @@ main(int argc, char *argv[]) appendPQExpBufferStr(pgdumpopts, " --binary-upgrade"); if (column_inserts) appendPQExpBufferStr(pgdumpopts, " --column-inserts"); + if (if_exists) + appendPQExpBufferStr(pgdumpopts, " --if-exists"); if (disable_dollar_quoting) appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting"); if (disable_triggers)2.
Spell check required:+ /* skip first n chars, and create a modifieble copy */
modifieble => modifiable
+ /* DROP IF EXISTS pattern is not appliable on dropStmt */
appliable => applicable
3.
+ /* + * Object description is based on dropStmt statement. But + * a drop statements can be enhanced about IF EXISTS clause. + * We have to increase a offset in this case, "IF EXISTS" + * should not be included on object description. + */Looks like you need to re-phrase these comments line. Something like:
/*
* Object description is based on dropStmt statement which may have
* IF EXISTS clause. Thus we need to update an offset such that it
* won't be included in the object description.
*/
Or as per your choice.Need to have careful thought on a bug mentioned above.
Thanks
On Fri, Jan 17, 2014 at 6:23 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
Hello
2014/1/16 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Hi Pavel,
I have reviewed the patch and here are my concerns and notes:
POSITIVES:
---
1. Patch applies with some white-space errors.
2. make / make install / make check is smooth. No issues as such.
3. Feature looks good as well.
4. NO concern on overall design.
5. Good work.NEGATIVES:
---Here are the points which I see in the review and would like you to have
your attention.1.
+ It use conditional commands (with <literal>IF EXISTS</literal>Grammar mistakes. use => uses
2. @@ -55,7 +55,8 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt, const int compression, ArchiveMode mode, SetupWorkerPtr setupWorkerPtr); static void _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH); -static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass); +static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, + bool isData, bool acl_pass); static char *replace_line_endings(const char *str); static void _doSetFixedOutputState(ArchiveHandle *AH); static void _doSetSessionAuth(ArchiveHandle *AH, const char *user); @@ -234,6 +235,7 @@ RestoreArchive(Archive *AHX) bool parallel_mode; TocEntry *te; OutputContext sav; +AH->stage = STAGE_INITIALIZING;
@@ -2961,7 +3005,8 @@ _getObjectDescription(PQExpBuffer buf, TocEntry
*te, ArchiveHandle *AH)
}static void -_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass) +_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, + bool acl_pass) { /* ACLs are dumped only during acl pass */ if (acl_pass)Above changes are NOT at all related to the patch. Please remove them
even
though they are clean-up like changes. Don't mix them with actual
changes.3.
+ if (strncmp(te->dropStmt + desc_len + 5, " IF
EXISTS", 9) != 0)" IF EXISTS" has 10 characters NOT 9.
4. + if (strncmp(te->dropStmt + desc_len + 5, " IF EXISTS", 9) != 0) + ahprintf(AH, "DROP %s IF EXISTS %s", + te->desc, + te->dropStmt + 6 + desc_len);Here you have used strncmp, starting at te->dropStmt + X,
where X = desc_len + 5. While adding back you used X = 6 + desc_len.
First time you used 5 as you added space in comparison but for adding
back we
want past space location and thus you have used 6. That's correct, but
little
bit confusing. Why not you simply used
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
to check whether drop statement has "IF EXISTS" or not like you did at
some
other place. This will remove my concern 3 and above confusion as well.
What you think ?5. + } + + elseExtra line before else part. Please remove it for consistency.
6. + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_dump) + printf(_(" --if-exists don't report error if cleaned object doesn't exist\n")); (pg_dumpall) + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_restore)Please have same message for all three.
7.
printf(_(" --binary-upgrade for use by upgrade
utilities only\n"));
printf(_(" --column-inserts dump data as INSERT
commands with column names\n"));
+ printf(_(" --if-exists don't report error if
cleaned object doesn't exist\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use
SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during
data-only restore\n"));Please maintain order like pg_dump and pg_restore. Also at variable
declaration
and at options parsing mechanism.I fixed a previous issue, see a attachment please
8. + if (if_exists && !outputClean) + exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");Are we really want to exit when -c is not provided ? Can't we simply
ignore
--if-exists in that case (or with emitting a warning) ?This behave is based on a talk related to proposal of this feature - and
I am thinking, this behave is little bit safer - ignoring requested
functionality is not what I like. And a error message is simple and clean
in this case - is not difficult to use it and it is not difficult to fix
missing option for userRegards
Pavel
Marking "Waiting on author".
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL CompanyPhone: +91 20 30589500
Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedbThis e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.
Attachments:
dump-restore-if-exists-opt-2014-01-25-2.patchtext/x-patch; charset=US-ASCII; name=dump-restore-if-exists-opt-2014-01-25-2.patchDownload
commit 0ad2dda3d5efe77cd0cbec9905bfe3fc36f084e1
Author: Pavel Stehule <pavel.stehule@gooddata.com>
Date: Sat Jan 25 13:38:22 2014 +0100
fixes related to review
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 8d45f24..c39767b 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -650,6 +650,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 5c6a101..ba6583d 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -301,6 +301,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--inserts</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index 717da42..55326d5 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -490,6 +490,16 @@
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-data-for-failed-tables</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 6927968..83f7216 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -113,6 +113,7 @@ typedef struct _restoreOptions
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 7fc0288..46d2b61 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -413,8 +413,59 @@ RestoreArchive(Archive *AHX)
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
- /* Drop it */
- ahprintf(AH, "%s", te->dropStmt);
+
+ if (*te->dropStmt != '\0')
+ {
+ /* Inject IF EXISTS clause to DROP part when it is required. */
+ if (ropt->if_exists)
+ {
+ char buffer[40];
+ char *mark;
+
+ /*
+ * Descriptor string should not be same as object
+ * specifier for DROP STATEMENT.
+ */
+ if (strcmp(te->desc, "DEFAULT") == 0)
+ {
+ mark = NULL;
+ }
+ else
+ {
+ if (strcmp(te->desc, "CONSTRAINT") == 0 ||
+ strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
+ strcmp(te->desc, "FK CONSTRAINT") == 0)
+ strcpy(buffer, "DROP CONSTRAINT");
+ else
+ snprintf(buffer, sizeof(buffer), "DROP %s",
+ te->desc);
+
+ mark = strstr(te->dropStmt, buffer);
+ }
+
+ if (mark != NULL)
+ {
+ size_t l = strlen(buffer);
+
+ /*
+ * Insert IF EXISTS clause when it is not
+ * used yet.
+ */
+ if (strncmp(mark + l, " IF EXISTS", 10) != 0)
+ {
+ *mark = '\0';
+
+ ahprintf(AH, "%s", te->dropStmt);
+ ahprintf(AH, "%s IF EXISTS", buffer);
+ ahprintf(AH, "%s", mark + l);
+ }
+ else
+ ahprintf(AH, "%s", te->dropStmt);
+ }
+ else
+ ahprintf(AH, "%s", te->dropStmt);
+ }
+ }
}
}
@@ -2942,9 +2993,38 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
- /* Chop "DROP " off the front and make a modifiable copy */
- char *first = pg_strdup(te->dropStmt + 5);
- char *last;
+ char *first;
+ char *last;
+
+ /*
+ * Object description is based on dropStmt statement which may have
+ * IF EXISTS clause. Thus we need to update an offset such that it
+ * won't be included in the object description.
+ */
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
+ {
+ char buffer[40];
+ size_t l;
+
+ /* IF EXISTS clause should be optional, check it*/
+ snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", type);
+ l = strlen(buffer);
+
+ if (strncmp(te->dropStmt, buffer, l) == 0)
+ {
+ /* append command type to target type */
+ appendPQExpBufferStr(buf, type);
+
+ /* skip first n chars, and create a modifiable copy */
+ first = pg_strdup(te->dropStmt + l);
+ }
+ else
+ /* DROP IF EXISTS pattern is not applicable on dropStmt */
+ first = pg_strdup(te->dropStmt + 5);
+ }
+ else
+ /* IF EXISTS clause was not used, simple solution */
+ first = pg_strdup(te->dropStmt + 5);
/* point to last character in string */
last = first + strlen(first) - 1;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 3862f05..1227d55 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -136,6 +136,7 @@ static int binary_upgrade = 0;
static int disable_dollar_quoting = 0;
static int dump_inserts = 0;
static int column_inserts = 0;
+static int if_exists = 0;
static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
@@ -349,6 +350,7 @@ main(int argc, char **argv)
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &dump_inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
@@ -577,6 +579,9 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
@@ -809,6 +814,7 @@ main(int argc, char **argv)
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
@@ -890,6 +896,7 @@ help(const char *progname)
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
@@ -2290,7 +2297,8 @@ dumpDatabase(Archive *fout)
}
- appendPQExpBuffer(delQry, "DROP DATABASE %s;\n",
+ appendPQExpBuffer(delQry, "DROP DATABASE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(datname));
dbDumpId = createDumpId();
@@ -7894,7 +7902,9 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
qnspname = pg_strdup(fmtId(nspinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname);
+ appendPQExpBuffer(delq, "DROP SCHEMA %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ qnspname);
appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname);
@@ -7953,7 +7963,9 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
qextname = pg_strdup(fmtId(extinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP EXTENSION %s;\n", qextname);
+ appendPQExpBuffer(delq, "DROP EXTENSION %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ qextname);
if (!binary_upgrade)
{
@@ -8127,7 +8139,8 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
* CASCADE shouldn't be required here as for normal types since the I/O
* functions are generic and do not get dropped.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -8257,7 +8270,8 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
* CASCADE shouldn't be required here as for normal types since the I/O
* functions are generic and do not get dropped.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -8596,7 +8610,8 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
* the type and its I/O functions makes it impossible to drop the type any
* other way.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s CASCADE;\n",
qtypname);
@@ -8855,7 +8870,8 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP DOMAIN %s.",
+ appendPQExpBuffer(delq, "DROP DOMAIN %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -9069,7 +9085,8 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -9379,7 +9396,8 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
else
lanschema = NULL;
- appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s;\n",
+ appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qlanname);
if (useParams)
@@ -9923,7 +9941,8 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP FUNCTION %s.%s;\n",
+ appendPQExpBuffer(delqry, "DROP FUNCTION %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(finfo->dobj.namespace->dobj.name),
funcsig);
@@ -10141,7 +10160,8 @@ dumpCast(Archive *fout, CastInfo *cast)
delqry = createPQExpBuffer();
labelq = createPQExpBuffer();
- appendPQExpBuffer(delqry, "DROP CAST (%s AS %s);\n",
+ appendPQExpBuffer(delqry, "DROP CAST %s(%s AS %s);\n",
+ if_exists ? "IF EXISTS " : "",
getFormattedTypeName(fout, cast->castsource, zeroAsNone),
getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
@@ -10411,7 +10431,8 @@ dumpOpr(Archive *fout, OprInfo *oprinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR %s.%s;\n",
+ appendPQExpBuffer(delq, "DROP OPERATOR %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(oprinfo->dobj.namespace->dobj.name),
oprid->data);
@@ -10696,7 +10717,8 @@ dumpOpclass(Archive *fout, OpclassInfo *opcinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s",
+ appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(opcinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s",
fmtId(opcinfo->dobj.name));
@@ -11140,7 +11162,8 @@ dumpOpfamily(Archive *fout, OpfamilyInfo *opfinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR FAMILY %s",
+ appendPQExpBuffer(delq, "DROP OPERATOR FAMILY %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(opfinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s",
fmtId(opfinfo->dobj.name));
@@ -11314,7 +11337,8 @@ dumpCollation(Archive *fout, CollInfo *collinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP COLLATION %s",
+ appendPQExpBuffer(delq, "DROP COLLATION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(collinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(collinfo->dobj.name));
@@ -11411,7 +11435,8 @@ dumpConversion(Archive *fout, ConvInfo *convinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP CONVERSION %s",
+ appendPQExpBuffer(delq, "DROP CONVERSION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(convinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(convinfo->dobj.name));
@@ -11723,7 +11748,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP AGGREGATE %s.%s;\n",
+ appendPQExpBuffer(delq, "DROP AGGREGATE %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(agginfo->aggfn.dobj.namespace->dobj.name),
aggsig);
@@ -11822,7 +11848,8 @@ dumpTSParser(Archive *fout, TSParserInfo *prsinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH PARSER %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH PARSER %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(prsinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(prsinfo->dobj.name));
@@ -11909,7 +11936,8 @@ dumpTSDictionary(Archive *fout, TSDictInfo *dictinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH DICTIONARY %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH DICTIONARY %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(dictinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(dictinfo->dobj.name));
@@ -11975,7 +12003,8 @@ dumpTSTemplate(Archive *fout, TSTemplateInfo *tmplinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH TEMPLATE %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH TEMPLATE %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(tmplinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(tmplinfo->dobj.name));
@@ -12103,7 +12132,8 @@ dumpTSConfig(Archive *fout, TSConfigInfo *cfginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH CONFIGURATION %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH CONFIGURATION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(cfginfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(cfginfo->dobj.name));
@@ -12179,7 +12209,8 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
appendPQExpBufferStr(q, ";\n");
- appendPQExpBuffer(delq, "DROP FOREIGN DATA WRAPPER %s;\n",
+ appendPQExpBuffer(delq, "DROP FOREIGN DATA WRAPPER %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qfdwname);
appendPQExpBuffer(labelq, "FOREIGN DATA WRAPPER %s",
@@ -12272,7 +12303,8 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
appendPQExpBufferStr(q, ";\n");
- appendPQExpBuffer(delq, "DROP SERVER %s;\n",
+ appendPQExpBuffer(delq, "DROP SERVER %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qsrvname);
appendPQExpBuffer(labelq, "SERVER %s", qsrvname);
@@ -12390,7 +12422,9 @@ dumpUserMappings(Archive *fout,
appendPQExpBufferStr(q, ";\n");
resetPQExpBuffer(delq);
- appendPQExpBuffer(delq, "DROP USER MAPPING FOR %s", fmtId(usename));
+ appendPQExpBuffer(delq, "DROP USER MAPPING %sFOR %s",
+ if_exists ? "IF EXISTS " : "",
+ fmtId(usename));
appendPQExpBuffer(delq, " SERVER %s;\n", fmtId(servername));
resetPQExpBuffer(tag);
@@ -12997,7 +13031,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP VIEW %s.",
+ appendPQExpBuffer(delq, "DROP VIEW %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -13074,7 +13109,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP %s %s.", reltypename,
+ appendPQExpBuffer(delq, "DROP %s %s%s.", reltypename,
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -13627,7 +13663,8 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -13743,7 +13780,8 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP INDEX %s.",
+ appendPQExpBuffer(delq, "DROP INDEX %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(indxinfo->dobj.name));
@@ -13864,7 +13902,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE ONLY %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %sONLY %s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -13897,7 +13936,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE ONLY %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %sONLY %s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -14166,7 +14206,8 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP SEQUENCE %s.",
+ appendPQExpBuffer(delqry, "DROP SEQUENCE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delqry, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -14362,7 +14403,8 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP TRIGGER %s ",
+ appendPQExpBuffer(delqry, "DROP TRIGGER %s%s ",
+ if_exists ? "IF EXISTS " : "",
fmtId(tginfo->dobj.name));
appendPQExpBuffer(delqry, "ON %s.",
fmtId(tbinfo->dobj.namespace->dobj.name));
@@ -14710,7 +14752,8 @@ dumpRule(Archive *fout, RuleInfo *rinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delcmd, "DROP RULE %s ",
+ appendPQExpBuffer(delcmd, "DROP RULE %s%s ",
+ if_exists ? "IF EXISTS " : "",
fmtId(rinfo->dobj.name));
appendPQExpBuffer(delcmd, "ON %s.",
fmtId(tbinfo->dobj.namespace->dobj.name));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 193c1a0..335fdde 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -73,6 +73,7 @@ static int binary_upgrade = 0;
static int column_inserts = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
+static int if_exists = 0;
static int inserts = 0;
static int no_tablespaces = 0;
static int use_setsessauth = 0;
@@ -119,6 +120,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &no_tablespaces, 1},
@@ -352,6 +354,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
appendPQExpBufferStr(pgdumpopts, " --disable-triggers");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (inserts)
appendPQExpBufferStr(pgdumpopts, " --inserts");
if (no_tablespaces)
@@ -564,6 +568,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 2648003..74f9c2a 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -76,6 +76,7 @@ main(int argc, char **argv)
Archive *AH;
char *inputFileSpec;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int no_data_for_failed_tables = 0;
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
@@ -116,6 +117,7 @@ main(int argc, char **argv)
* the following options don't have an equivalent short option letter
*/
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
@@ -342,6 +344,14 @@ main(int argc, char **argv)
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ if (if_exists && !opts->dropSchema)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+ opts->if_exists = if_exists;
+
if (opts->formatName)
{
switch (opts->formatName[0])
@@ -456,6 +466,7 @@ usage(const char *progname)
printf(_(" -x, --no-privileges skip restoration of access privileges (grant/revoke)\n"));
printf(_(" -1, --single-transaction restore as a single transaction\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
Sorry second update
fixed dump if-exists of DROP OPERATOR
Regards
Pavel
2014-01-25 Pavel Stehule <pavel.stehule@gmail.com>
Show quoted text
Hello
I fixed all described issues. There was a more mistakes, that I fixed -
main mistake was in work with te->desc variable.You propose a regress tests for pg_dump? I searched in mailing lists and
there was some proposals about it. I am not against, but I have to do some
research and better be this as separate patch.This patch requires b152c6cd0de1827ba58756e24e18110cf902182a commit.
Regards
Pavel
2014-01-21 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Hi Pavel,
Consider following test scenario:
mydb=# \d emp
Table "public.emp"
Column | Type | Modifiers
--------+---------+-----------
empno | integer | not null
deptno | integer |
ename | text |
Indexes:
"emp_pkey" PRIMARY KEY, btree (empno)
Foreign-key constraints:
"emp_deptno_fkey" FOREIGN KEY (deptno) REFERENCES dept(deptno)mydb=# \d dept
Table "public.dept"
Column | Type | Modifiers
--------+---------+-----------
deptno | integer | not null
dname | text |
Indexes:
"dept_pkey" PRIMARY KEY, btree (deptno)
Referenced by:
TABLE "emp" CONSTRAINT "emp_deptno_fkey" FOREIGN KEY (deptno)
REFERENCES dept(deptno)mydb=# \q
jeevan@ubuntu:~/pg_master$ ./install/bin/pg_dump -d mydb --if-exists -cmydb_ic.dmp
I see following lines in dump which looks certainly wrong:
===DROP FK CONSTRAINT IF EXISTS ublic.emp DROP CONSTRAINT emp_deptno_fkey;
DROP CONSTRAINT IF EXISTS Y public.emp DROP CONSTRAINT emp_pkey;
DROP CONSTRAINT IF EXISTS Y public.dept DROP CONSTRAINT dept_pkey;When try to restore, as expected it is throwing an error:
===psql:mydb_ic.dmp:14: ERROR: syntax error at or near "FK"
LINE 1: DROP FK CONSTRAINT IF EXISTS ublic.emp DROP CONSTRAINT emp_d...
^
psql:mydb_ic.dmp:15: ERROR: syntax error at or near "CONSTRAINT"
LINE 1: DROP CONSTRAINT IF EXISTS Y public.emp DROP CONSTRAINT emp_p...
^
psql:mydb_ic.dmp:16: ERROR: syntax error at or near "CONSTRAINT"
LINE 1: DROP CONSTRAINT IF EXISTS Y public.dept DROP CONSTRAINT dept...
^Note:
===
Commands which are in form of ALTER TABLE ... DROP are failing.
You need to test each and every object with DROP .. IF EXISTS command.
Better write small test-case with all objects included.Following logic has flaw: === diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 7fc0288..0677712 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -413,8 +413,30 @@ RestoreArchive(Archive *AHX) /* Select owner and schema as necessary */ _becomeOwner(AH, te); _selectOutputSchema(AH, te->namespace); - /* Drop it */ - ahprintf(AH, "%s", te->dropStmt); + + if (*te->dropStmt != '\0') + { + /* Inject IF EXISTS clause when it is required. */ + if (ropt->if_exists) + { + char buffer[40]; + size_t l; + + /* But do it only, when it is not there yet. */ + snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", + te->desc); + l = strlen(buffer); + + if (strncmp(te->dropStmt, buffer, l) != 0) + {+ ahprintf(AH, "DROP %s IF EXISTS %s", + te->desc, + te->dropStmt + l); + } + else + ahprintf(AH, "%s", te->dropStmt); + } + } } }Also:
===1.
This is still out of sync.@@ -348,6 +350,8 @@ main(int argc, char *argv[]) appendPQExpBufferStr(pgdumpopts, " --binary-upgrade"); if (column_inserts) appendPQExpBufferStr(pgdumpopts, " --column-inserts"); + if (if_exists) + appendPQExpBufferStr(pgdumpopts, " --if-exists"); if (disable_dollar_quoting) appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting"); if (disable_triggers)2.
Spell check required:+ /* skip first n chars, and create a modifieble copy */
modifieble => modifiable
+ /* DROP IF EXISTS pattern is not appliable on dropStmt */
appliable => applicable
3.
+ /* + * Object description is based on dropStmt statement. But + * a drop statements can be enhanced about IF EXISTS clause. + * We have to increase a offset in this case, "IF EXISTS" + * should not be included on object description. + */Looks like you need to re-phrase these comments line. Something like:
/*
* Object description is based on dropStmt statement which may
have
* IF EXISTS clause. Thus we need to update an offset such that
it
* won't be included in the object description.
*/
Or as per your choice.Need to have careful thought on a bug mentioned above.
Thanks
On Fri, Jan 17, 2014 at 6:23 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
Hello
2014/1/16 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Hi Pavel,
I have reviewed the patch and here are my concerns and notes:
POSITIVES:
---
1. Patch applies with some white-space errors.
2. make / make install / make check is smooth. No issues as such.
3. Feature looks good as well.
4. NO concern on overall design.
5. Good work.NEGATIVES:
---Here are the points which I see in the review and would like you to
have your attention.1.
+ It use conditional commands (with <literal>IF EXISTS</literal>Grammar mistakes. use => uses
2. @@ -55,7 +55,8 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt, const int compression, ArchiveMode mode, SetupWorkerPtr setupWorkerPtr); static void _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH); -static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass); +static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, + bool isData, bool acl_pass); static char *replace_line_endings(const char *str); static void _doSetFixedOutputState(ArchiveHandle *AH); static void _doSetSessionAuth(ArchiveHandle *AH, const char *user); @@ -234,6 +235,7 @@ RestoreArchive(Archive *AHX) bool parallel_mode; TocEntry *te; OutputContext sav; +AH->stage = STAGE_INITIALIZING;
@@ -2961,7 +3005,8 @@ _getObjectDescription(PQExpBuffer buf, TocEntry
*te, ArchiveHandle *AH)
}static void -_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass) +_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, + bool acl_pass) { /* ACLs are dumped only during acl pass */ if (acl_pass)Above changes are NOT at all related to the patch. Please remove them
even
though they are clean-up like changes. Don't mix them with actual
changes.3.
+ if (strncmp(te->dropStmt + desc_len + 5, " IF
EXISTS", 9) != 0)" IF EXISTS" has 10 characters NOT 9.
4. + if (strncmp(te->dropStmt + desc_len + 5, " IF EXISTS", 9) != 0) + ahprintf(AH, "DROP %s IF EXISTS %s", + te->desc, + te->dropStmt + 6 + desc_len);Here you have used strncmp, starting at te->dropStmt + X,
where X = desc_len + 5. While adding back you used X = 6 + desc_len.
First time you used 5 as you added space in comparison but for adding
back we
want past space location and thus you have used 6. That's correct, but
little
bit confusing. Why not you simply used
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
to check whether drop statement has "IF EXISTS" or not like you did at
some
other place. This will remove my concern 3 and above confusion as well.
What you think ?5. + } + + elseExtra line before else part. Please remove it for consistency.
6. + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_dump) + printf(_(" --if-exists don't report error if cleaned object doesn't exist\n")); (pg_dumpall) + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_restore)Please have same message for all three.
7.
printf(_(" --binary-upgrade for use by upgrade
utilities only\n"));
printf(_(" --column-inserts dump data as INSERT
commands with column names\n"));
+ printf(_(" --if-exists don't report error if
cleaned object doesn't exist\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting,
use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during
data-only restore\n"));Please maintain order like pg_dump and pg_restore. Also at variable
declaration
and at options parsing mechanism.I fixed a previous issue, see a attachment please
8. + if (if_exists && !outputClean) + exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");Are we really want to exit when -c is not provided ? Can't we simply
ignore
--if-exists in that case (or with emitting a warning) ?This behave is based on a talk related to proposal of this feature - and
I am thinking, this behave is little bit safer - ignoring requested
functionality is not what I like. And a error message is simple and clean
in this case - is not difficult to use it and it is not difficult to fix
missing option for userRegards
Pavel
Marking "Waiting on author".
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL CompanyPhone: +91 20 30589500
Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedbThis e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.
Attachments:
dump-restore-if-exists-opt-2014-01-25-3.patchtext/x-patch; charset=US-ASCII; name=dump-restore-if-exists-opt-2014-01-25-3.patchDownload
commit ccf1d230edc569c1a27c41065511269ee99898bd
Author: Pavel Stehule <pavel.stehule@gooddata.com>
Date: Sat Jan 25 13:38:22 2014 +0100
fixes related to review
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 8d45f24..c39767b 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -650,6 +650,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 5c6a101..ba6583d 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -301,6 +301,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--inserts</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index 717da42..55326d5 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -490,6 +490,16 @@
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-data-for-failed-tables</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 6927968..83f7216 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -113,6 +113,7 @@ typedef struct _restoreOptions
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 7fc0288..b1bf72c 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -413,8 +413,63 @@ RestoreArchive(Archive *AHX)
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
- /* Drop it */
- ahprintf(AH, "%s", te->dropStmt);
+
+ if (*te->dropStmt != '\0')
+ {
+ /* Inject IF EXISTS clause to DROP part when it is required. */
+ if (ropt->if_exists)
+ {
+ char buffer[40];
+ char *mark;
+
+ /*
+ * Descriptor string should not be same as object
+ * specifier for DROP STATEMENT.
+ */
+ if (strcmp(te->desc, "DEFAULT") == 0)
+ {
+ mark = NULL;
+ }
+ else
+ {
+ if (strcmp(te->desc, "CONSTRAINT") == 0 ||
+ strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
+ strcmp(te->desc, "FK CONSTRAINT") == 0)
+ strcpy(buffer, "DROP CONSTRAINT");
+ else if (strcmp(te->desc, "OPERATOR") == 0 ||
+ strcmp(te->desc, "OPERATOR CLASS") == 0 ||
+ strcmp(te->desc, "OPERATOR FAMILY") == 0)
+ strcmp(buffer, "DROP OPERATOR");
+ else
+ snprintf(buffer, sizeof(buffer), "DROP %s",
+ te->desc);
+
+ mark = strstr(te->dropStmt, buffer);
+ }
+
+ if (mark != NULL)
+ {
+ size_t l = strlen(buffer);
+
+ /*
+ * Insert IF EXISTS clause when it is not
+ * used yet.
+ */
+ if (strncmp(mark + l, " IF EXISTS", 10) != 0)
+ {
+ *mark = '\0';
+
+ ahprintf(AH, "%s", te->dropStmt);
+ ahprintf(AH, "%s IF EXISTS", buffer);
+ ahprintf(AH, "%s", mark + l);
+ }
+ else
+ ahprintf(AH, "%s", te->dropStmt);
+ }
+ else
+ ahprintf(AH, "%s", te->dropStmt);
+ }
+ }
}
}
@@ -2942,9 +2997,49 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
- /* Chop "DROP " off the front and make a modifiable copy */
- char *first = pg_strdup(te->dropStmt + 5);
- char *last;
+ char *first;
+ char *last;
+
+ /*
+ * Object description is based on dropStmt statement which may have
+ * IF EXISTS clause. Thus we need to update an offset such that it
+ * won't be included in the object description.
+ */
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
+ {
+ char buffer[40];
+ size_t l;
+ char *supertype;
+
+ /*
+ * Be sure, so IF EXISTS is used as DROP stmt option.
+ * Use only first word from type var. CLASS or FAMILY
+ * should not be used here.
+ */
+ if (strncmp(type, "OPERATOR", 8) == 0)
+ supertype = "OPERATOR";
+ else
+ supertype = type;
+
+ snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", supertype);
+
+ l = strlen(buffer);
+
+ if (strncmp(te->dropStmt, buffer, l) == 0)
+ {
+ /* append command type to target type */
+ appendPQExpBufferStr(buf, supertype);
+
+ /* skip first n chars, and create a modifiable copy */
+ first = pg_strdup(te->dropStmt + l);
+ }
+ else
+ /* DROP IF EXISTS pattern is not applicable on dropStmt */
+ first = pg_strdup(te->dropStmt + 5);
+ }
+ else
+ /* IF EXISTS clause was not used, simple solution */
+ first = pg_strdup(te->dropStmt + 5);
/* point to last character in string */
last = first + strlen(first) - 1;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 3862f05..1227d55 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -136,6 +136,7 @@ static int binary_upgrade = 0;
static int disable_dollar_quoting = 0;
static int dump_inserts = 0;
static int column_inserts = 0;
+static int if_exists = 0;
static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
@@ -349,6 +350,7 @@ main(int argc, char **argv)
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &dump_inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
@@ -577,6 +579,9 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
@@ -809,6 +814,7 @@ main(int argc, char **argv)
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
@@ -890,6 +896,7 @@ help(const char *progname)
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
@@ -2290,7 +2297,8 @@ dumpDatabase(Archive *fout)
}
- appendPQExpBuffer(delQry, "DROP DATABASE %s;\n",
+ appendPQExpBuffer(delQry, "DROP DATABASE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(datname));
dbDumpId = createDumpId();
@@ -7894,7 +7902,9 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
qnspname = pg_strdup(fmtId(nspinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname);
+ appendPQExpBuffer(delq, "DROP SCHEMA %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ qnspname);
appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname);
@@ -7953,7 +7963,9 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
qextname = pg_strdup(fmtId(extinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP EXTENSION %s;\n", qextname);
+ appendPQExpBuffer(delq, "DROP EXTENSION %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ qextname);
if (!binary_upgrade)
{
@@ -8127,7 +8139,8 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
* CASCADE shouldn't be required here as for normal types since the I/O
* functions are generic and do not get dropped.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -8257,7 +8270,8 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
* CASCADE shouldn't be required here as for normal types since the I/O
* functions are generic and do not get dropped.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -8596,7 +8610,8 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
* the type and its I/O functions makes it impossible to drop the type any
* other way.
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s CASCADE;\n",
qtypname);
@@ -8855,7 +8870,8 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP DOMAIN %s.",
+ appendPQExpBuffer(delq, "DROP DOMAIN %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -9069,7 +9085,8 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TYPE %s.",
+ appendPQExpBuffer(delq, "DROP TYPE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tyinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
qtypname);
@@ -9379,7 +9396,8 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
else
lanschema = NULL;
- appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s;\n",
+ appendPQExpBuffer(delqry, "DROP PROCEDURAL LANGUAGE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qlanname);
if (useParams)
@@ -9923,7 +9941,8 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP FUNCTION %s.%s;\n",
+ appendPQExpBuffer(delqry, "DROP FUNCTION %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(finfo->dobj.namespace->dobj.name),
funcsig);
@@ -10141,7 +10160,8 @@ dumpCast(Archive *fout, CastInfo *cast)
delqry = createPQExpBuffer();
labelq = createPQExpBuffer();
- appendPQExpBuffer(delqry, "DROP CAST (%s AS %s);\n",
+ appendPQExpBuffer(delqry, "DROP CAST %s(%s AS %s);\n",
+ if_exists ? "IF EXISTS " : "",
getFormattedTypeName(fout, cast->castsource, zeroAsNone),
getFormattedTypeName(fout, cast->casttarget, zeroAsNone));
@@ -10411,7 +10431,8 @@ dumpOpr(Archive *fout, OprInfo *oprinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR %s.%s;\n",
+ appendPQExpBuffer(delq, "DROP OPERATOR %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(oprinfo->dobj.namespace->dobj.name),
oprid->data);
@@ -10696,7 +10717,8 @@ dumpOpclass(Archive *fout, OpclassInfo *opcinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s",
+ appendPQExpBuffer(delq, "DROP OPERATOR CLASS %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(opcinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s",
fmtId(opcinfo->dobj.name));
@@ -11140,7 +11162,8 @@ dumpOpfamily(Archive *fout, OpfamilyInfo *opfinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP OPERATOR FAMILY %s",
+ appendPQExpBuffer(delq, "DROP OPERATOR FAMILY %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(opfinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s",
fmtId(opfinfo->dobj.name));
@@ -11314,7 +11337,8 @@ dumpCollation(Archive *fout, CollInfo *collinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP COLLATION %s",
+ appendPQExpBuffer(delq, "DROP COLLATION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(collinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(collinfo->dobj.name));
@@ -11411,7 +11435,8 @@ dumpConversion(Archive *fout, ConvInfo *convinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP CONVERSION %s",
+ appendPQExpBuffer(delq, "DROP CONVERSION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(convinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(convinfo->dobj.name));
@@ -11723,7 +11748,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP AGGREGATE %s.%s;\n",
+ appendPQExpBuffer(delq, "DROP AGGREGATE %s%s.%s;\n",
+ if_exists ? "IF EXISTS " : "",
fmtId(agginfo->aggfn.dobj.namespace->dobj.name),
aggsig);
@@ -11822,7 +11848,8 @@ dumpTSParser(Archive *fout, TSParserInfo *prsinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH PARSER %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH PARSER %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(prsinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(prsinfo->dobj.name));
@@ -11909,7 +11936,8 @@ dumpTSDictionary(Archive *fout, TSDictInfo *dictinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH DICTIONARY %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH DICTIONARY %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(dictinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(dictinfo->dobj.name));
@@ -11975,7 +12003,8 @@ dumpTSTemplate(Archive *fout, TSTemplateInfo *tmplinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH TEMPLATE %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH TEMPLATE %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(tmplinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(tmplinfo->dobj.name));
@@ -12103,7 +12132,8 @@ dumpTSConfig(Archive *fout, TSConfigInfo *cfginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "DROP TEXT SEARCH CONFIGURATION %s",
+ appendPQExpBuffer(delq, "DROP TEXT SEARCH CONFIGURATION %s%s",
+ if_exists ? "IF EXISTS " : "",
fmtId(cfginfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, ".%s;\n",
fmtId(cfginfo->dobj.name));
@@ -12179,7 +12209,8 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
appendPQExpBufferStr(q, ";\n");
- appendPQExpBuffer(delq, "DROP FOREIGN DATA WRAPPER %s;\n",
+ appendPQExpBuffer(delq, "DROP FOREIGN DATA WRAPPER %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qfdwname);
appendPQExpBuffer(labelq, "FOREIGN DATA WRAPPER %s",
@@ -12272,7 +12303,8 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
appendPQExpBufferStr(q, ";\n");
- appendPQExpBuffer(delq, "DROP SERVER %s;\n",
+ appendPQExpBuffer(delq, "DROP SERVER %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
qsrvname);
appendPQExpBuffer(labelq, "SERVER %s", qsrvname);
@@ -12390,7 +12422,9 @@ dumpUserMappings(Archive *fout,
appendPQExpBufferStr(q, ";\n");
resetPQExpBuffer(delq);
- appendPQExpBuffer(delq, "DROP USER MAPPING FOR %s", fmtId(usename));
+ appendPQExpBuffer(delq, "DROP USER MAPPING %sFOR %s",
+ if_exists ? "IF EXISTS " : "",
+ fmtId(usename));
appendPQExpBuffer(delq, " SERVER %s;\n", fmtId(servername));
resetPQExpBuffer(tag);
@@ -12997,7 +13031,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP VIEW %s.",
+ appendPQExpBuffer(delq, "DROP VIEW %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -13074,7 +13109,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP %s %s.", reltypename,
+ appendPQExpBuffer(delq, "DROP %s %s%s.", reltypename,
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -13627,7 +13663,8 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -13743,7 +13780,8 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "DROP INDEX %s.",
+ appendPQExpBuffer(delq, "DROP INDEX %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s;\n",
fmtId(indxinfo->dobj.name));
@@ -13864,7 +13902,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE ONLY %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %sONLY %s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -13897,7 +13936,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* DROP must be fully qualified in case same name appears in
* pg_catalog
*/
- appendPQExpBuffer(delq, "ALTER TABLE ONLY %s.",
+ appendPQExpBuffer(delq, "ALTER TABLE %sONLY %s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delq, "%s ",
fmtId(tbinfo->dobj.name));
@@ -14166,7 +14206,8 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP SEQUENCE %s.",
+ appendPQExpBuffer(delqry, "DROP SEQUENCE %s%s.",
+ if_exists ? "IF EXISTS " : "",
fmtId(tbinfo->dobj.namespace->dobj.name));
appendPQExpBuffer(delqry, "%s;\n",
fmtId(tbinfo->dobj.name));
@@ -14362,7 +14403,8 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delqry, "DROP TRIGGER %s ",
+ appendPQExpBuffer(delqry, "DROP TRIGGER %s%s ",
+ if_exists ? "IF EXISTS " : "",
fmtId(tginfo->dobj.name));
appendPQExpBuffer(delqry, "ON %s.",
fmtId(tbinfo->dobj.namespace->dobj.name));
@@ -14710,7 +14752,8 @@ dumpRule(Archive *fout, RuleInfo *rinfo)
/*
* DROP must be fully qualified in case same name appears in pg_catalog
*/
- appendPQExpBuffer(delcmd, "DROP RULE %s ",
+ appendPQExpBuffer(delcmd, "DROP RULE %s%s ",
+ if_exists ? "IF EXISTS " : "",
fmtId(rinfo->dobj.name));
appendPQExpBuffer(delcmd, "ON %s.",
fmtId(tbinfo->dobj.namespace->dobj.name));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 193c1a0..335fdde 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -73,6 +73,7 @@ static int binary_upgrade = 0;
static int column_inserts = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
+static int if_exists = 0;
static int inserts = 0;
static int no_tablespaces = 0;
static int use_setsessauth = 0;
@@ -119,6 +120,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &no_tablespaces, 1},
@@ -352,6 +354,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
appendPQExpBufferStr(pgdumpopts, " --disable-triggers");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (inserts)
appendPQExpBufferStr(pgdumpopts, " --inserts");
if (no_tablespaces)
@@ -564,6 +568,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 2648003..74f9c2a 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -76,6 +76,7 @@ main(int argc, char **argv)
Archive *AH;
char *inputFileSpec;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int no_data_for_failed_tables = 0;
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
@@ -116,6 +117,7 @@ main(int argc, char **argv)
* the following options don't have an equivalent short option letter
*/
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
@@ -342,6 +344,14 @@ main(int argc, char **argv)
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ if (if_exists && !opts->dropSchema)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+ opts->if_exists = if_exists;
+
if (opts->formatName)
{
switch (opts->formatName[0])
@@ -456,6 +466,7 @@ usage(const char *progname)
printf(_(" -x, --no-privileges skip restoration of access privileges (grant/revoke)\n"));
printf(_(" -1, --single-transaction restore as a single transaction\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
Hello
Second update - I reduced patch by removing not necessary changes.
Attached tests and Makefile
Now --if-exists option is fully consistent with -c option
With some free time I plan to enhance test script about more object types -
now It contains almost all usual types.
Regards
Pavel
2014-01-21 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Show quoted text
Hi Pavel,
Consider following test scenario:
mydb=# \d emp
Table "public.emp"
Column | Type | Modifiers
--------+---------+-----------
empno | integer | not null
deptno | integer |
ename | text |
Indexes:
"emp_pkey" PRIMARY KEY, btree (empno)
Foreign-key constraints:
"emp_deptno_fkey" FOREIGN KEY (deptno) REFERENCES dept(deptno)mydb=# \d dept
Table "public.dept"
Column | Type | Modifiers
--------+---------+-----------
deptno | integer | not null
dname | text |
Indexes:
"dept_pkey" PRIMARY KEY, btree (deptno)
Referenced by:
TABLE "emp" CONSTRAINT "emp_deptno_fkey" FOREIGN KEY (deptno)
REFERENCES dept(deptno)mydb=# \q
jeevan@ubuntu:~/pg_master$ ./install/bin/pg_dump -d mydb --if-exists -c >
mydb_ic.dmpI see following lines in dump which looks certainly wrong:
===DROP FK CONSTRAINT IF EXISTS ublic.emp DROP CONSTRAINT emp_deptno_fkey;
DROP CONSTRAINT IF EXISTS Y public.emp DROP CONSTRAINT emp_pkey;
DROP CONSTRAINT IF EXISTS Y public.dept DROP CONSTRAINT dept_pkey;When try to restore, as expected it is throwing an error:
===psql:mydb_ic.dmp:14: ERROR: syntax error at or near "FK"
LINE 1: DROP FK CONSTRAINT IF EXISTS ublic.emp DROP CONSTRAINT emp_d...
^
psql:mydb_ic.dmp:15: ERROR: syntax error at or near "CONSTRAINT"
LINE 1: DROP CONSTRAINT IF EXISTS Y public.emp DROP CONSTRAINT emp_p...
^
psql:mydb_ic.dmp:16: ERROR: syntax error at or near "CONSTRAINT"
LINE 1: DROP CONSTRAINT IF EXISTS Y public.dept DROP CONSTRAINT dept...
^Note:
===
Commands which are in form of ALTER TABLE ... DROP are failing.
You need to test each and every object with DROP .. IF EXISTS command.
Better write small test-case with all objects included.Following logic has flaw: === diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 7fc0288..0677712 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -413,8 +413,30 @@ RestoreArchive(Archive *AHX) /* Select owner and schema as necessary */ _becomeOwner(AH, te); _selectOutputSchema(AH, te->namespace); - /* Drop it */ - ahprintf(AH, "%s", te->dropStmt); + + if (*te->dropStmt != '\0') + { + /* Inject IF EXISTS clause when it is required. */ + if (ropt->if_exists) + { + char buffer[40]; + size_t l; + + /* But do it only, when it is not there yet. */ + snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", + te->desc); + l = strlen(buffer); + + if (strncmp(te->dropStmt, buffer, l) != 0) + {+ ahprintf(AH, "DROP %s IF EXISTS %s", + te->desc, + te->dropStmt + l); + } + else + ahprintf(AH, "%s", te->dropStmt); + } + } } }Also:
===1.
This is still out of sync.@@ -348,6 +350,8 @@ main(int argc, char *argv[]) appendPQExpBufferStr(pgdumpopts, " --binary-upgrade"); if (column_inserts) appendPQExpBufferStr(pgdumpopts, " --column-inserts"); + if (if_exists) + appendPQExpBufferStr(pgdumpopts, " --if-exists"); if (disable_dollar_quoting) appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting"); if (disable_triggers)2.
Spell check required:+ /* skip first n chars, and create a modifieble copy */
modifieble => modifiable
+ /* DROP IF EXISTS pattern is not appliable on dropStmt */
appliable => applicable
3.
+ /* + * Object description is based on dropStmt statement. But + * a drop statements can be enhanced about IF EXISTS clause. + * We have to increase a offset in this case, "IF EXISTS" + * should not be included on object description. + */Looks like you need to re-phrase these comments line. Something like:
/*
* Object description is based on dropStmt statement which may have
* IF EXISTS clause. Thus we need to update an offset such that it
* won't be included in the object description.
*/
Or as per your choice.Need to have careful thought on a bug mentioned above.
Thanks
On Fri, Jan 17, 2014 at 6:23 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
Hello
2014/1/16 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Hi Pavel,
I have reviewed the patch and here are my concerns and notes:
POSITIVES:
---
1. Patch applies with some white-space errors.
2. make / make install / make check is smooth. No issues as such.
3. Feature looks good as well.
4. NO concern on overall design.
5. Good work.NEGATIVES:
---Here are the points which I see in the review and would like you to have
your attention.1.
+ It use conditional commands (with <literal>IF EXISTS</literal>Grammar mistakes. use => uses
2. @@ -55,7 +55,8 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt, const int compression, ArchiveMode mode, SetupWorkerPtr setupWorkerPtr); static void _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH); -static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass); +static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, + bool isData, bool acl_pass); static char *replace_line_endings(const char *str); static void _doSetFixedOutputState(ArchiveHandle *AH); static void _doSetSessionAuth(ArchiveHandle *AH, const char *user); @@ -234,6 +235,7 @@ RestoreArchive(Archive *AHX) bool parallel_mode; TocEntry *te; OutputContext sav; +AH->stage = STAGE_INITIALIZING;
@@ -2961,7 +3005,8 @@ _getObjectDescription(PQExpBuffer buf, TocEntry
*te, ArchiveHandle *AH)
}static void -_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass) +_printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, + bool acl_pass) { /* ACLs are dumped only during acl pass */ if (acl_pass)Above changes are NOT at all related to the patch. Please remove them
even
though they are clean-up like changes. Don't mix them with actual
changes.3.
+ if (strncmp(te->dropStmt + desc_len + 5, " IF
EXISTS", 9) != 0)" IF EXISTS" has 10 characters NOT 9.
4. + if (strncmp(te->dropStmt + desc_len + 5, " IF EXISTS", 9) != 0) + ahprintf(AH, "DROP %s IF EXISTS %s", + te->desc, + te->dropStmt + 6 + desc_len);Here you have used strncmp, starting at te->dropStmt + X,
where X = desc_len + 5. While adding back you used X = 6 + desc_len.
First time you used 5 as you added space in comparison but for adding
back we
want past space location and thus you have used 6. That's correct, but
little
bit confusing. Why not you simply used
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
to check whether drop statement has "IF EXISTS" or not like you did at
some
other place. This will remove my concern 3 and above confusion as well.
What you think ?5. + } + + elseExtra line before else part. Please remove it for consistency.
6. + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_dump) + printf(_(" --if-exists don't report error if cleaned object doesn't exist\n")); (pg_dumpall) + printf(_(" --if-exists use IF EXISTS when dropping objects\n")); (pg_restore)Please have same message for all three.
7.
printf(_(" --binary-upgrade for use by upgrade
utilities only\n"));
printf(_(" --column-inserts dump data as INSERT
commands with column names\n"));
+ printf(_(" --if-exists don't report error if
cleaned object doesn't exist\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use
SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during
data-only restore\n"));Please maintain order like pg_dump and pg_restore. Also at variable
declaration
and at options parsing mechanism.I fixed a previous issue, see a attachment please
8. + if (if_exists && !outputClean) + exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");Are we really want to exit when -c is not provided ? Can't we simply
ignore
--if-exists in that case (or with emitting a warning) ?This behave is based on a talk related to proposal of this feature - and
I am thinking, this behave is little bit safer - ignoring requested
functionality is not what I like. And a error message is simple and clean
in this case - is not difficult to use it and it is not difficult to fix
missing option for userRegards
Pavel
Marking "Waiting on author".
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL CompanyPhone: +91 20 30589500
Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedbThis e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.
Attachments:
dump-restore-if-exists-opt-2014-01-26-1.patchtext/x-patch; charset=US-ASCII; name=dump-restore-if-exists-opt-2014-01-26-1.patchDownload
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
***************
*** 650,655 ****
--- 650,665 ----
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
*** a/doc/src/sgml/ref/pg_dumpall.sgml
--- b/doc/src/sgml/ref/pg_dumpall.sgml
***************
*** 301,306 ****
--- 301,316 ----
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--inserts</option></term>
<listitem>
<para>
*** a/doc/src/sgml/ref/pg_restore.sgml
--- b/doc/src/sgml/ref/pg_restore.sgml
***************
*** 490,495 ****
--- 490,505 ----
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-data-for-failed-tables</option></term>
<listitem>
<para>
*** a/src/bin/pg_dump/pg_backup.h
--- b/src/bin/pg_dump/pg_backup.h
***************
*** 113,118 ****
--- 113,119 ----
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
***************
*** 413,420 ****
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
! /* Drop it */
! ahprintf(AH, "%s", te->dropStmt);
}
}
--- 413,493 ----
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
!
! if (*te->dropStmt != '\0')
! {
! /* Inject IF EXISTS clause to DROP part when it is required. */
! if (ropt->if_exists)
! {
! char buffer[40];
! char *mark;
! char *dropStmt = te->dropStmt;
! PQExpBuffer ftStmt = createPQExpBuffer();
!
! /* ALTER TABLE should be enahnced to ALTER TABLE IF EXISTS */
! if (strncmp(dropStmt, "ALTER TABLE", 11) == 0)
! {
! /*
! * Prepare fault tolerant statement, but
! * ensure unique IF EXISTS option.
! */
! if (strncmp(dropStmt + 11, " IF EXISTS", 10) != 0)
! {
! appendPQExpBuffer(ftStmt, "ALTER TABLE IF EXISTS");
! dropStmt = dropStmt + 11;
! }
! }
!
! /*
! * Descriptor string (te-desc) should not be same as object
! * specifier for DROP STATEMENT. The DROP DEFAULT has not
! * IF EXISTS clause - has not sense.
! */
! if (strcmp(te->desc, "DEFAULT") != 0)
! {
! if (strcmp(te->desc, "CONSTRAINT") == 0 ||
! strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
! strcmp(te->desc, "FK CONSTRAINT") == 0)
! strcpy(buffer, "DROP CONSTRAINT");
! else
! snprintf(buffer, sizeof(buffer), "DROP %s",
! te->desc);
!
! mark = strstr(dropStmt, buffer);
! }
! else
! mark = NULL;
!
! if (mark != NULL)
! {
! size_t l = strlen(buffer);
!
! /*
! * Insert IF EXISTS clause when it is not
! * used yet.
! */
! if (strncmp(mark + l, " IF EXISTS", 10) != 0)
! {
! *mark = '\0';
!
! appendPQExpBuffer(ftStmt, "%s%s IF EXISTS%s",
! dropStmt,
! buffer,
! mark + l);
! }
! else
! appendPQExpBuffer(ftStmt, "%s", dropStmt);
! }
! else
! appendPQExpBuffer(ftStmt, "%s", dropStmt);
!
! ahprintf(AH, "%s", ftStmt->data);
!
! destroyPQExpBuffer(ftStmt);
! }
! else
! ahprintf(AH, "%s", te->dropStmt);
! }
}
}
***************
*** 2942,2950 ****
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
! /* Chop "DROP " off the front and make a modifiable copy */
! char *first = pg_strdup(te->dropStmt + 5);
! char *last;
/* point to last character in string */
last = first + strlen(first) - 1;
--- 3015,3053 ----
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
! char *first;
! char *last;
!
! /*
! * Object description is based on dropStmt statement which may have
! * IF EXISTS clause. Thus we need to update an offset such that it
! * won't be included in the object description.
! */
! if (strstr(te->dropStmt, "IF EXISTS") != NULL)
! {
! char buffer[40];
! size_t l;
!
! /* Be sure, so IF EXISTS is used as DROP stmt option. */
! snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", type);
!
! l = strlen(buffer);
!
! if (strncmp(te->dropStmt, buffer, l) == 0)
! {
! /* append command type to target type */
! appendPQExpBufferStr(buf, type);
!
! /* skip first n chars, and create a modifiable copy */
! first = pg_strdup(te->dropStmt + l);
! }
! else
! /* DROP IF EXISTS pattern is not applicable on dropStmt */
! first = pg_strdup(te->dropStmt + 5);
! }
! else
! /* IF EXISTS clause was not used, simple solution */
! first = pg_strdup(te->dropStmt + 5);
/* point to last character in string */
last = first + strlen(first) - 1;
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
***************
*** 136,141 ****
--- 136,142 ----
static int disable_dollar_quoting = 0;
static int dump_inserts = 0;
static int column_inserts = 0;
+ static int if_exists = 0;
static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
***************
*** 349,354 ****
--- 350,356 ----
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &dump_inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
***************
*** 577,582 ****
--- 579,587 ----
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
***************
*** 809,814 ****
--- 814,820 ----
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
***************
*** 890,895 ****
--- 896,902 ----
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
*** a/src/bin/pg_dump/pg_dumpall.c
--- b/src/bin/pg_dump/pg_dumpall.c
***************
*** 73,78 ****
--- 73,79 ----
static int column_inserts = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int inserts = 0;
static int no_tablespaces = 0;
static int use_setsessauth = 0;
***************
*** 119,124 ****
--- 120,126 ----
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &no_tablespaces, 1},
***************
*** 352,357 ****
--- 354,361 ----
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
appendPQExpBufferStr(pgdumpopts, " --disable-triggers");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (inserts)
appendPQExpBufferStr(pgdumpopts, " --inserts");
if (no_tablespaces)
***************
*** 564,569 ****
--- 568,574 ----
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
*** a/src/bin/pg_dump/pg_restore.c
--- b/src/bin/pg_dump/pg_restore.c
***************
*** 76,81 ****
--- 76,82 ----
Archive *AH;
char *inputFileSpec;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int no_data_for_failed_tables = 0;
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
***************
*** 116,121 ****
--- 117,123 ----
* the following options don't have an equivalent short option letter
*/
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
***************
*** 342,347 ****
--- 344,357 ----
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ if (if_exists && !opts->dropSchema)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+ opts->if_exists = if_exists;
+
if (opts->formatName)
{
switch (opts->formatName[0])
***************
*** 456,461 ****
--- 466,472 ----
printf(_(" -x, --no-privileges skip restoration of access privileges (grant/revoke)\n"));
printf(_(" -1, --single-transaction restore as a single transaction\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
Hi Pavel,
Now the patch looks good to me. However when I try to restore your own sql
file's dump, I get following errors:
pg_restore: [archiver (db)] could not execute query: ERROR: relation
"public.emp" does not exist
Command was: DROP TRIGGER IF EXISTS emp_insert_trigger ON public.emp;
pg_restore: [archiver (db)] could not execute query: ERROR: schema
"myschema" does not exist
Command was: DROP FUNCTION IF EXISTS myschema.int_to_date(integer);
Is that expected after your patch ?
Also, I didn't quite understand these lines of comments:
/*
* Descriptor string (te-desc) should not be same
as object
* specifier for DROP STATEMENT. The DROP DEFAULT
has not
* IF EXISTS clause - has not sense.
*/
Will you please rephrase ?
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
2014-01-29 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Hi Pavel,
Now the patch looks good to me. However when I try to restore your own sql
file's dump, I get following errors:pg_restore: [archiver (db)] could not execute query: ERROR: relation
"public.emp" does not exist
Command was: DROP TRIGGER IF EXISTS emp_insert_trigger ON public.emp;pg_restore: [archiver (db)] could not execute query: ERROR: schema
"myschema" does not exist
Command was: DROP FUNCTION IF EXISTS myschema.int_to_date(integer);Is that expected after your patch ?
it should be fixed by
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b152c6cd0de1827ba58756e24e18110cf902182acommit
Also, I didn't quite understand these lines of comments:
/*
* Descriptor string (te-desc) should not be same
as object
* specifier for DROP STATEMENT. The DROP DEFAULT
has not
* IF EXISTS clause - has not sense.
*/Will you please rephrase ?
I can try it - .
A content of te->desc is usually substring of DROP STATEMENT with one
related exception - CONSTRAINT.
Independent to previous sentence - ALTER TABLE ALTER COLUMN DROP DEFAULT
doesn't support IF EXISTS - and therefore it should not be injected.
Regards
Pavel
Show quoted text
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
2014-01-29 Pavel Stehule <pavel.stehule@gmail.com>
2014-01-29 Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Hi Pavel,
Now the patch looks good to me. However when I try to restore your own
sql file's dump, I get following errors:pg_restore: [archiver (db)] could not execute query: ERROR: relation
"public.emp" does not exist
Command was: DROP TRIGGER IF EXISTS emp_insert_trigger ON public.emp;pg_restore: [archiver (db)] could not execute query: ERROR: schema
"myschema" does not exist
Command was: DROP FUNCTION IF EXISTS myschema.int_to_date(integer);Is that expected after your patch ?
it should be fixed by
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b152c6cd0de1827ba58756e24e18110cf902182acommitAlso, I didn't quite understand these lines of comments:
/*
* Descriptor string (te-desc) should not be same
as object
* specifier for DROP STATEMENT. The DROP DEFAULT
has not
* IF EXISTS clause - has not sense.
*/Will you please rephrase ?
I can try it - .
A content of te->desc is usually substring of DROP STATEMENT with one
related exception - CONSTRAINT.
Independent to previous sentence - ALTER TABLE ALTER COLUMN DROP DEFAULT
doesn't support IF EXISTS - and therefore it should not be injected.
is it ok?
Show quoted text
Regards
Pavel
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Hi Pavel,
it should be fixed by
Ok. Good.
Sorry I didn't update my sources. Done now. Thanks
Also, I didn't quite understand these lines of comments:
/*
* Descriptor string (te-desc) should not be
same as object
* specifier for DROP STATEMENT. The DROP
DEFAULT has not
* IF EXISTS clause - has not sense.
*/Will you please rephrase ?
I can try it - .
A content of te->desc is usually substring of DROP STATEMENT with one
related exception - CONSTRAINT.
Independent to previous sentence - ALTER TABLE ALTER COLUMN DROP DEFAULT
doesn't support IF EXISTS - and therefore it should not be injected.is it ok?
Fine with me.
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Hello
patch with updated comment
regards
Pavel
2014-01-30 Jeevan Chalke <jeevan.chalke@enterprisedb.com>:
Show quoted text
Hi Pavel,
it should be fixed by
Ok. Good.
Sorry I didn't update my sources. Done now. ThanksAlso, I didn't quite understand these lines of comments:
/*
* Descriptor string (te-desc) should not be
same as object
* specifier for DROP STATEMENT. The DROP
DEFAULT has not
* IF EXISTS clause - has not sense.
*/Will you please rephrase ?
I can try it - .
A content of te->desc is usually substring of DROP STATEMENT with one
related exception - CONSTRAINT.
Independent to previous sentence - ALTER TABLE ALTER COLUMN DROP DEFAULT
doesn't support IF EXISTS - and therefore it should not be injected.is it ok?
Fine with me.
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Attachments:
dump-restore-if-exists-opt-2014-01-30-1.patchtext/x-patch; charset=US-ASCII; name=dump-restore-if-exists-opt-2014-01-30-1.patchDownload
commit e72e3ae9003d2c8eea0e687122a2f31d21674b81
Author: Pavel Stehule <pavel.stehule@gooddata.com>
Date: Thu Jan 30 11:28:40 2014 +0100
fix comment
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
new file mode 100644
index 8d45f24..c39767b
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
*************** PostgreSQL documentation
*** 650,655 ****
--- 650,665 ----
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
new file mode 100644
index 5c6a101..ba6583d
*** a/doc/src/sgml/ref/pg_dumpall.sgml
--- b/doc/src/sgml/ref/pg_dumpall.sgml
*************** PostgreSQL documentation
*** 301,306 ****
--- 301,316 ----
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--inserts</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
new file mode 100644
index 717da42..55326d5
*** a/doc/src/sgml/ref/pg_restore.sgml
--- b/doc/src/sgml/ref/pg_restore.sgml
***************
*** 490,495 ****
--- 490,505 ----
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-data-for-failed-tables</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
new file mode 100644
index 6927968..83f7216
*** a/src/bin/pg_dump/pg_backup.h
--- b/src/bin/pg_dump/pg_backup.h
*************** typedef struct _restoreOptions
*** 113,118 ****
--- 113,119 ----
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
new file mode 100644
index 7fc0288..a7061d3
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** RestoreArchive(Archive *AHX)
*** 413,420 ****
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
! /* Drop it */
! ahprintf(AH, "%s", te->dropStmt);
}
}
--- 413,496 ----
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
!
! if (*te->dropStmt != '\0')
! {
! /* Inject IF EXISTS clause to DROP part when it is required. */
! if (ropt->if_exists)
! {
! char buffer[40];
! char *mark;
! char *dropStmt = te->dropStmt;
! PQExpBuffer ftStmt = createPQExpBuffer();
!
! /* ALTER TABLE should be enahnced to ALTER TABLE IF EXISTS */
! if (strncmp(dropStmt, "ALTER TABLE", 11) == 0)
! {
! /*
! * Prepare fault tolerant statement, but
! * ensure unique IF EXISTS option.
! */
! if (strncmp(dropStmt + 11, " IF EXISTS", 10) != 0)
! {
! appendPQExpBuffer(ftStmt, "ALTER TABLE IF EXISTS");
! dropStmt = dropStmt + 11;
! }
! }
!
! /*
! * A content of te->desc is usually substring of DROP STATEMENT
! * with one related exception - CONSTRAINTs.
! *
! * Independent to previous sentence - ALTER TABLE ALTER COLUMN
! * DROP DEFAULT doesn't support IF EXISTS - and therefore it
! * should not be injected.
! */
! if (strcmp(te->desc, "DEFAULT") != 0)
! {
! if (strcmp(te->desc, "CONSTRAINT") == 0 ||
! strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
! strcmp(te->desc, "FK CONSTRAINT") == 0)
! strcpy(buffer, "DROP CONSTRAINT");
! else
! snprintf(buffer, sizeof(buffer), "DROP %s",
! te->desc);
!
! mark = strstr(dropStmt, buffer);
! }
! else
! mark = NULL;
!
! if (mark != NULL)
! {
! size_t l = strlen(buffer);
!
! /*
! * Insert IF EXISTS clause when it is not
! * used yet.
! */
! if (strncmp(mark + l, " IF EXISTS", 10) != 0)
! {
! *mark = '\0';
!
! appendPQExpBuffer(ftStmt, "%s%s IF EXISTS%s",
! dropStmt,
! buffer,
! mark + l);
! }
! else
! appendPQExpBuffer(ftStmt, "%s", dropStmt);
! }
! else
! appendPQExpBuffer(ftStmt, "%s", dropStmt);
!
! ahprintf(AH, "%s", ftStmt->data);
!
! destroyPQExpBuffer(ftStmt);
! }
! else
! ahprintf(AH, "%s", te->dropStmt);
! }
}
}
*************** _getObjectDescription(PQExpBuffer buf, T
*** 2942,2950 ****
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
! /* Chop "DROP " off the front and make a modifiable copy */
! char *first = pg_strdup(te->dropStmt + 5);
! char *last;
/* point to last character in string */
last = first + strlen(first) - 1;
--- 3018,3056 ----
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
! char *first;
! char *last;
!
! /*
! * Object description is based on dropStmt statement which may have
! * IF EXISTS clause. Thus we need to update an offset such that it
! * won't be included in the object description.
! */
! if (strstr(te->dropStmt, "IF EXISTS") != NULL)
! {
! char buffer[40];
! size_t l;
!
! /* Be sure, so IF EXISTS is used as DROP stmt option. */
! snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", type);
!
! l = strlen(buffer);
!
! if (strncmp(te->dropStmt, buffer, l) == 0)
! {
! /* append command type to target type */
! appendPQExpBufferStr(buf, type);
!
! /* skip first n chars, and create a modifiable copy */
! first = pg_strdup(te->dropStmt + l);
! }
! else
! /* DROP IF EXISTS pattern is not applicable on dropStmt */
! first = pg_strdup(te->dropStmt + 5);
! }
! else
! /* IF EXISTS clause was not used, simple solution */
! first = pg_strdup(te->dropStmt + 5);
/* point to last character in string */
last = first + strlen(first) - 1;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index ebbb5b7..2f4f3a1
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** static int binary_upgrade = 0;
*** 136,141 ****
--- 136,142 ----
static int disable_dollar_quoting = 0;
static int dump_inserts = 0;
static int column_inserts = 0;
+ static int if_exists = 0;
static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
*************** main(int argc, char **argv)
*** 349,354 ****
--- 350,356 ----
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &dump_inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
*************** main(int argc, char **argv)
*** 577,582 ****
--- 579,587 ----
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
*************** main(int argc, char **argv)
*** 809,814 ****
--- 814,820 ----
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
*************** help(const char *progname)
*** 890,895 ****
--- 896,902 ----
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
new file mode 100644
index 193c1a0..335fdde
*** a/src/bin/pg_dump/pg_dumpall.c
--- b/src/bin/pg_dump/pg_dumpall.c
*************** static int binary_upgrade = 0;
*** 73,78 ****
--- 73,79 ----
static int column_inserts = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int inserts = 0;
static int no_tablespaces = 0;
static int use_setsessauth = 0;
*************** main(int argc, char *argv[])
*** 119,124 ****
--- 120,126 ----
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &no_tablespaces, 1},
*************** main(int argc, char *argv[])
*** 352,357 ****
--- 354,361 ----
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
appendPQExpBufferStr(pgdumpopts, " --disable-triggers");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (inserts)
appendPQExpBufferStr(pgdumpopts, " --inserts");
if (no_tablespaces)
*************** help(void)
*** 564,569 ****
--- 568,574 ----
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
new file mode 100644
index 2648003..74f9c2a
*** a/src/bin/pg_dump/pg_restore.c
--- b/src/bin/pg_dump/pg_restore.c
*************** main(int argc, char **argv)
*** 76,81 ****
--- 76,82 ----
Archive *AH;
char *inputFileSpec;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int no_data_for_failed_tables = 0;
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
*************** main(int argc, char **argv)
*** 116,121 ****
--- 117,123 ----
* the following options don't have an equivalent short option letter
*/
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
*************** main(int argc, char **argv)
*** 342,347 ****
--- 344,357 ----
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ if (if_exists && !opts->dropSchema)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+ opts->if_exists = if_exists;
+
if (opts->formatName)
{
switch (opts->formatName[0])
*************** usage(const char *progname)
*** 456,461 ****
--- 466,472 ----
printf(_(" -x, --no-privileges skip restoration of access privileges (grant/revoke)\n"));
printf(_(" -1, --single-transaction restore as a single transaction\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
Hi Pavel,
Now patch looks good to me and I think it is in good shape to pass it on to
the committer as well.
However, I have
- Tweaked few comments
- Removed white-space errors
- Fixed typos
- Fixed indentation
Attached patch with my changes. However entire design and code logic is
untouched.
Please have a quick look and pass it on to committor if you have no issues
OR
ask me to assign it to the committor, NO issues either way.
Feel free to reject my changes if you didn't like them.
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Attachments:
dump-restore-if-exists-opt-2014-01-30-Jeevan.patchtext/x-diff; charset=US-ASCII; name=dump-restore-if-exists-opt-2014-01-30-Jeevan.patchDownload
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 8d45f24..c39767b 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -650,6 +650,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 5c6a101..ba6583d 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -301,6 +301,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--inserts</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index 717da42..55326d5 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -490,6 +490,16 @@
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-data-for-failed-tables</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 6927968..83f7216 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -113,6 +113,7 @@ typedef struct _restoreOptions
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 7fc0288..c08a0d3 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -413,8 +413,84 @@ RestoreArchive(Archive *AHX)
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
- /* Drop it */
- ahprintf(AH, "%s", te->dropStmt);
+
+ if (*te->dropStmt != '\0')
+ {
+ /* Inject IF EXISTS clause to DROP part when required. */
+ if (ropt->if_exists)
+ {
+ char buffer[40];
+ char *mark;
+ char *dropStmt = te->dropStmt;
+ PQExpBuffer ftStmt = createPQExpBuffer();
+
+ /*
+ * Need to inject IF EXISTS clause after ALTER TABLE
+ * part in ALTER TABLE .. DROP statement
+ */
+ if (strncmp(dropStmt, "ALTER TABLE", 11) == 0)
+ {
+ /*
+ * Prepare fault tolerant statement, but ensure
+ * unique IF EXISTS option.
+ */
+ if (strncmp(dropStmt + 11, " IF EXISTS", 10) != 0)
+ {
+ appendPQExpBuffer(ftStmt,
+ "ALTER TABLE IF EXISTS");
+ dropStmt = dropStmt + 11;
+ }
+ }
+
+ /*
+ * A content of te->desc is usually substring of a DROP
+ * statement with one related exception - CONSTRAINTs.
+ *
+ * Independent to previous sentence, ALTER TABLE ..
+ * ALTER COLUMN .. DROP DEFAULT statement does not
+ * support IF EXISTS clause and therefore it should not
+ * be injected.
+ */
+ if (strcmp(te->desc, "DEFAULT") != 0)
+ {
+ if (strcmp(te->desc, "CONSTRAINT") == 0 ||
+ strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
+ strcmp(te->desc, "FK CONSTRAINT") == 0)
+ strcpy(buffer, "DROP CONSTRAINT");
+ else
+ snprintf(buffer, sizeof(buffer), "DROP %s",
+ te->desc);
+
+ mark = strstr(dropStmt, buffer);
+ }
+ else
+ mark = NULL;
+
+ if (mark != NULL)
+ {
+ size_t l = strlen(buffer);
+
+ /* Inject IF EXISTS clause if not alredy present */
+ if (strncmp(mark + l, " IF EXISTS", 10) != 0)
+ {
+ *mark = '\0';
+
+ appendPQExpBuffer(ftStmt, "%s%s IF EXISTS%s",
+ dropStmt, buffer, mark + l);
+ }
+ else
+ appendPQExpBuffer(ftStmt, "%s", dropStmt);
+ }
+ else
+ appendPQExpBuffer(ftStmt, "%s", dropStmt);
+
+ ahprintf(AH, "%s", ftStmt->data);
+
+ destroyPQExpBuffer(ftStmt);
+ }
+ else
+ ahprintf(AH, "%s", te->dropStmt);
+ }
}
}
@@ -2942,9 +3018,39 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
- /* Chop "DROP " off the front and make a modifiable copy */
- char *first = pg_strdup(te->dropStmt + 5);
- char *last;
+ char *first;
+ char *last;
+
+ /*
+ * Object description is based on dropStmt statement which may have
+ * IF EXISTS clause. Thus we need to update an offset such that it
+ * won't be included in the object description.
+ */
+ if (strstr(te->dropStmt, "IF EXISTS") != NULL)
+ {
+ char buffer[40];
+ size_t l;
+
+ /* Be sure, so IF EXISTS is used as DROP stmt option. */
+ snprintf(buffer, sizeof(buffer), "DROP %s IF EXISTS", type);
+
+ l = strlen(buffer);
+
+ if (strncmp(te->dropStmt, buffer, l) == 0)
+ {
+ /* append command type to target type */
+ appendPQExpBufferStr(buf, type);
+
+ /* skip first n chars, and create a modifiable copy */
+ first = pg_strdup(te->dropStmt + l);
+ }
+ else
+ /* DROP IF EXISTS pattern is not applicable on dropStmt */
+ first = pg_strdup(te->dropStmt + 5);
+ }
+ else
+ /* IF EXISTS clause was not used, simple solution */
+ first = pg_strdup(te->dropStmt + 5);
/* point to last character in string */
last = first + strlen(first) - 1;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ebbb5b7..2f4f3a1 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -136,6 +136,7 @@ static int binary_upgrade = 0;
static int disable_dollar_quoting = 0;
static int dump_inserts = 0;
static int column_inserts = 0;
+static int if_exists = 0;
static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
@@ -349,6 +350,7 @@ main(int argc, char **argv)
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &dump_inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
@@ -577,6 +579,9 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
@@ -809,6 +814,7 @@ main(int argc, char **argv)
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
@@ -890,6 +896,7 @@ help(const char *progname)
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 193c1a0..335fdde 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -73,6 +73,7 @@ static int binary_upgrade = 0;
static int column_inserts = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
+static int if_exists = 0;
static int inserts = 0;
static int no_tablespaces = 0;
static int use_setsessauth = 0;
@@ -119,6 +120,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &no_tablespaces, 1},
@@ -352,6 +354,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
appendPQExpBufferStr(pgdumpopts, " --disable-triggers");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (inserts)
appendPQExpBufferStr(pgdumpopts, " --inserts");
if (no_tablespaces)
@@ -564,6 +568,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 2648003..74f9c2a 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -76,6 +76,7 @@ main(int argc, char **argv)
Archive *AH;
char *inputFileSpec;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int no_data_for_failed_tables = 0;
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
@@ -116,6 +117,7 @@ main(int argc, char **argv)
* the following options don't have an equivalent short option letter
*/
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
@@ -342,6 +344,14 @@ main(int argc, char **argv)
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ if (if_exists && !opts->dropSchema)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+ opts->if_exists = if_exists;
+
if (opts->formatName)
{
switch (opts->formatName[0])
@@ -456,6 +466,7 @@ usage(const char *progname)
printf(_(" -x, --no-privileges skip restoration of access privileges (grant/revoke)\n"));
printf(_(" -1, --single-transaction restore as a single transaction\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
Hello
All is ok
Thank you
Pavel
2014-01-30 Jeevan Chalke <jeevan.chalke@enterprisedb.com>:
Show quoted text
Hi Pavel,
Now patch looks good to me and I think it is in good shape to pass it on to
the committer as well.However, I have
- Tweaked few comments
- Removed white-space errors
- Fixed typos
- Fixed indentationAttached patch with my changes. However entire design and code logic is
untouched.Please have a quick look and pass it on to committor if you have no issues
OR
ask me to assign it to the committor, NO issues either way.Feel free to reject my changes if you didn't like them.
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
OK.
Assigned it to committer.
Thanks for the hard work.
On Thu, Jan 30, 2014 at 6:16 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
Hello
All is ok
Thank you
Pavel
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
2014-01-30 Jeevan Chalke <jeevan.chalke@enterprisedb.com>:
OK.
Assigned it to committer.
Thanks for the hard work.
Thank you for review
Pavel
Show quoted text
On Thu, Jan 30, 2014 at 6:16 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
Hello
All is ok
Thank you
Pavel
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Hi Peter,
I am not sure why you getting build unstable due to white-space errors.
Are you referring to these line ?
*11:25:01* src/bin/pg_dump/pg_backup_archiver.c:477: indent with
spaces.*11:25:01* + dropStmt,*11:25:01*
src/bin/pg_dump/pg_backup_archiver.c:478: indent with
spaces.*11:25:10* + buffer,*11:25:14*
src/bin/pg_dump/pg_backup_archiver.c:479: indent with
spaces.*11:25:15* + mark + l);*11:25:15* + echo
unstable
If yes, then in my latest attached patch, these lines are NOT AT ALL there.
I have informed on my comment that I have fixed these in my version of
patch,
but still you got unstable build. NOT sure how. Seems like you are applying
wrong patch.
Will you please let us know what's going wrong ?
Thanks
On Thu, Jan 30, 2014 at 6:56 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
2014-01-30 Jeevan Chalke <jeevan.chalke@enterprisedb.com>:
OK.
Assigned it to committer.
Thanks for the hard work.
Thank you for review
Pavel
On Thu, Jan 30, 2014 at 6:16 PM, Pavel Stehule <pavel.stehule@gmail.com>wrote:
Hello
All is ok
Thank you
Pavel
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Phone: +91 20 30589500
Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb
This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.
Jeevan Chalke escribi�:
If yes, then in my latest attached patch, these lines are NOT AT ALL there.
I have informed on my comment that I have fixed these in my version of
patch,
but still you got unstable build. NOT sure how. Seems like you are applying
wrong patch.Will you please let us know what's going wrong ?
The commitfest app is not a chat area. When you add new versions of a
patch, please mark them as "patch" (not "comment") and make sure to
provide the message-id of the latest version.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Jeevan Chalke escribi�:
I don't understand this code. (Well, it's pg_dump.) Or maybe I do
understand it, and it's not doing what you think it's doing. I mean, in
this part:
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 7fc0288..c08a0d3 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -413,8 +413,84 @@ RestoreArchive(Archive *AHX) /* Select owner and schema as necessary */ _becomeOwner(AH, te); _selectOutputSchema(AH, te->namespace); - /* Drop it */ - ahprintf(AH, "%s", te->dropStmt); + + if (*te->dropStmt != '\0') + { + /* Inject IF EXISTS clause to DROP part when required. */ + if (ropt->if_exists)
It does *not* modify te->dropStmt, it only sends ahprint() a different
version of what was stored (injected the wanted IF EXISTS clause). If
that is correct, then why are we, in this other part, trying to remove
the IF EXISTS clause?
@@ -2942,9 +3018,39 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH) strcmp(type, "OPERATOR CLASS") == 0 || strcmp(type, "OPERATOR FAMILY") == 0) { - /* Chop "DROP " off the front and make a modifiable copy */ - char *first = pg_strdup(te->dropStmt + 5); - char *last; + char *first; + char *last; + + /* + * Object description is based on dropStmt statement which may have + * IF EXISTS clause. Thus we need to update an offset such that it + * won't be included in the object description. + */
Maybe I am mistaken and the te->dropStmt already contains the IF EXISTS
bit for some reason; but if so I don't know why that is. Care to
explain?
I also think that _getObjectDescription() becomes overworked after this
patch. I wonder if we should be storing te->objIdentity so that we can
construct the ALTER OWNER command without going to as much trouble as
parsing the DROP command. Is there a way to do that? Maybe we can ask
the server for the object identity, for example. There is a new
function to do that in 9.3 which perhaps we can now use.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello
2014-02-17 18:10 GMT+01:00 Alvaro Herrera <alvherre@2ndquadrant.com>:
Jeevan Chalke escribió:
I don't understand this code. (Well, it's pg_dump.) Or maybe I do
understand it, and it's not doing what you think it's doing. I mean, in
this part:diff --git a/src/bin/pg_dump/pg_backup_archiver.cb/src/bin/pg_dump/pg_backup_archiver.c
index 7fc0288..c08a0d3 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -413,8 +413,84 @@ RestoreArchive(Archive *AHX) /* Select owner and schema as necessary */ _becomeOwner(AH, te); _selectOutputSchema(AH, te->namespace); - /* Drop it */ - ahprintf(AH, "%s", te->dropStmt); + + if (*te->dropStmt != '\0') + { + /* Inject IF EXISTS clause to DROPpart when required. */
+ if (ropt->if_exists)
It does *not* modify te->dropStmt, it only sends ahprint() a different
version of what was stored (injected the wanted IF EXISTS clause). If
that is correct, then why are we, in this other part, trying to remove
the IF EXISTS clause?
we should not to modify te->dropStmt, because only in this fragment a DROP
STATEMENT is produced. This additional logic ensures correct syntax for all
variation of DROP.
When I wrote this patch I had a initial problem with understanding relation
between pg_dump and pg_restore. And I pushed IF EXISTS to all related DROP
statements producers. But I was wrong. All the drop statements are reparsed
and transformed and serialized in this fragment. So only this fragment
should be modified. IF EXISTS clause can be injected before, when you read
plain text dump (produced by pg_dump --if-exists) in pg_restore.
@@ -2942,9 +3018,39 @@ _getObjectDescription(PQExpBuffer buf, TocEntry
*te, ArchiveHandle *AH)
strcmp(type, "OPERATOR CLASS") == 0 || strcmp(type, "OPERATOR FAMILY") == 0) { - /* Chop "DROP " off the front and make a modifiable copy */ - char *first = pg_strdup(te->dropStmt + 5); - char *last; + char *first; + char *last; + + /* + * Object description is based on dropStmt statement whichmay have
+ * IF EXISTS clause. Thus we need to update an offset
such that it
+ * won't be included in the object description. + */Maybe I am mistaken and the te->dropStmt already contains the IF EXISTS
bit for some reason; but if so I don't know why that is. Care to
explain?
pg_restore is available to read plain dump produced by pg_dump --if-exists.
It is way how IF EXISTS can infect te->dropStmt
I also think that _getObjectDescription() becomes overworked after this
patch. I wonder if we should be storing te->objIdentity so that we can
construct the ALTER OWNER command without going to as much trouble as
parsing the DROP command. Is there a way to do that? Maybe we can ask
the server for the object identity, for example. There is a new
function to do that in 9.3 which perhaps we can now use.
do you think a pg_describe_object function?
Probably it is possible, but its significantly much more invasive change,
you should to get objidentity, that is not trivial
Regards
Pavel
Show quoted text
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Pavel Stehule escribi�:
2014-02-17 18:10 GMT+01:00 Alvaro Herrera <alvherre@2ndquadrant.com>:
Maybe I am mistaken and the te->dropStmt already contains the IF EXISTS
bit for some reason; but if so I don't know why that is. Care to
explain?pg_restore is available to read plain dump produced by pg_dump --if-exists.
It is way how IF EXISTS can infect te->dropStmt
Makes sense, I guess.
I also think that _getObjectDescription() becomes overworked after this
patch. I wonder if we should be storing te->objIdentity so that we can
construct the ALTER OWNER command without going to as much trouble as
parsing the DROP command. Is there a way to do that? Maybe we can ask
the server for the object identity, for example. There is a new
function to do that in 9.3 which perhaps we can now use.do you think a pg_describe_object function?
Probably it is possible, but its significantly much more invasive change,
you should to get objidentity, that is not trivial
I was thinking in pg_identify_object(). It can be given the values used
to construct the CatalogId of each tocEntry.
But yes, it is more invasive.
I'd guess that would be a project related to cleaning up the ALTER
OWNER. What we have now looks like an kludge.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Feb 17, 2014 at 7:43 PM, Alvaro Herrera <alvherre@2ndquadrant.com>wrote:
Jeevan Chalke escribió:
If yes, then in my latest attached patch, these lines are NOT AT ALL
there.
I have informed on my comment that I have fixed these in my version of
patch,
but still you got unstable build. NOT sure how. Seems like you areapplying
wrong patch.
Will you please let us know what's going wrong ?
The commitfest app is not a chat area.
Hmm. Extremely sorry about that.
When you add new versions of a
patch, please mark them as "patch" (not "comment") and make sure to
provide the message-id of the latest version.
Ohh, I was needed to mark it as patch and NOT comment (with message id).
And since I had marked it as comment, commitfest app was taking previous
patch
and not the latest one.
My bad. Will keep this in mind.
Thanks
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Hello
2014-02-17 22:14 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hello
2014-02-17 18:10 GMT+01:00 Alvaro Herrera <alvherre@2ndquadrant.com>:
Jeevan Chalke escribió:
I don't understand this code. (Well, it's pg_dump.) Or maybe I do
understand it, and it's not doing what you think it's doing. I mean, in
this part:diff --git a/src/bin/pg_dump/pg_backup_archiver.cb/src/bin/pg_dump/pg_backup_archiver.c
index 7fc0288..c08a0d3 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -413,8 +413,84 @@ RestoreArchive(Archive *AHX) /* Select owner and schema as necessary */ _becomeOwner(AH, te); _selectOutputSchema(AH, te->namespace); - /* Drop it */ - ahprintf(AH, "%s", te->dropStmt); + + if (*te->dropStmt != '\0') + { + /* Inject IF EXISTS clause toDROP part when required. */
+ if (ropt->if_exists)
It does *not* modify te->dropStmt, it only sends ahprint() a different
version of what was stored (injected the wanted IF EXISTS clause). If
that is correct, then why are we, in this other part, trying to remove
the IF EXISTS clause?we should not to modify te->dropStmt, because only in this fragment a DROP
STATEMENT is produced. This additional logic ensures correct syntax for all
variation of DROP.When I wrote this patch I had a initial problem with understanding
relation between pg_dump and pg_restore. And I pushed IF EXISTS to all
related DROP statements producers. But I was wrong. All the drop statements
are reparsed and transformed and serialized in this fragment. So only this
fragment should be modified. IF EXISTS clause can be injected before, when
you read plain text dump (produced by pg_dump --if-exists) in pg_restore.
I was wrong - "IF EXISTS" was there, because I generated DROP IF EXISTS
elsewhere in some very old stages of this patch. It is useless to check it
there now.
@@ -2942,9 +3018,39 @@ _getObjectDescription(PQExpBuffer buf, TocEntry
*te, ArchiveHandle *AH)
strcmp(type, "OPERATOR CLASS") == 0 ||
strcmp(type, "OPERATOR FAMILY") == 0)
{
- /* Chop "DROP " off the front and make a modifiable copy*/
- char *first = pg_strdup(te->dropStmt + 5); - char *last; + char *first; + char *last; + + /* + * Object description is based on dropStmt statementwhich may have
+ * IF EXISTS clause. Thus we need to update an offset
such that it
+ * won't be included in the object description. + */Maybe I am mistaken and the te->dropStmt already contains the IF EXISTS
bit for some reason; but if so I don't know why that is. Care to
explain?pg_restore is available to read plain dump produced by pg_dump
--if-exists. It is way how IF EXISTS can infect te->dropStmtI also think that _getObjectDescription() becomes overworked after this
patch. I wonder if we should be storing te->objIdentity so that we can
construct the ALTER OWNER command without going to as much trouble as
parsing the DROP command. Is there a way to do that? Maybe we can ask
the server for the object identity, for example. There is a new
function to do that in 9.3 which perhaps we can now use.do you think a pg_describe_object function?
Probably it is possible, but its significantly much more invasive change,
you should to get objidentity, that is not trivialRegards
It is irony, so this is death code - it is not used now. So I removed it
from patch.
Reduced, fixed patch attached + used tests
Regards
Pavel
Show quoted text
Pavel
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
dump-restore-if-exists-opt-2014-02-18-1.patchtext/x-patch; charset=US-ASCII; name=dump-restore-if-exists-opt-2014-02-18-1.patchDownload
commit 937830b60920a8fac84cd2adb24c3d1b5280b036
Author: Pavel Stehule <pavel.stehule@gooddata.com>
Date: Tue Feb 18 14:25:40 2014 +0100
reduced2
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 8d45f24..c39767b 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -650,6 +650,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 5c6a101..ba6583d 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -301,6 +301,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--inserts</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index 717da42..55326d5 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -490,6 +490,16 @@
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ It uses conditional commands (with <literal>IF EXISTS</literal>
+ clause) for cleaning database schema.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-data-for-failed-tables</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 6927968..83f7216 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -113,6 +113,7 @@ typedef struct _restoreOptions
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 2b36e45..f36caf2 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -413,8 +413,64 @@ RestoreArchive(Archive *AHX)
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
- /* Drop it */
- ahprintf(AH, "%s", te->dropStmt);
+
+ if (*te->dropStmt != '\0')
+ {
+ /* Inject IF EXISTS clause to DROP part when required. */
+ if (ropt->if_exists)
+ {
+ char buffer[40];
+ char *mark;
+ char *dropStmt = te->dropStmt;
+ PQExpBuffer ftStmt = createPQExpBuffer();
+
+ /*
+ * Need to inject IF EXISTS clause after ALTER TABLE
+ * part in ALTER TABLE .. DROP statement
+ */
+ if (strncmp(dropStmt, "ALTER TABLE", 11) == 0)
+ {
+ appendPQExpBuffer(ftStmt,
+ "ALTER TABLE IF EXISTS");
+ dropStmt = dropStmt + 11;
+ }
+
+ /*
+ * A content of te->desc is usually substring of a DROP
+ * statement with one related exception - CONSTRAINTs.
+ *
+ * Independent to previous sentence, ALTER TABLE ..
+ * ALTER COLUMN .. DROP DEFAULT statement does not
+ * support IF EXISTS clause and therefore it should not
+ * be injected.
+ */
+ if (strcmp(te->desc, "DEFAULT") != 0)
+ {
+ if (strcmp(te->desc, "CONSTRAINT") == 0 ||
+ strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
+ strcmp(te->desc, "FK CONSTRAINT") == 0)
+ strcpy(buffer, "DROP CONSTRAINT");
+ else
+ snprintf(buffer, sizeof(buffer), "DROP %s",
+ te->desc);
+
+ mark = strstr(dropStmt, buffer);
+ Assert(mark != NULL);
+
+ *mark = '\0';
+ appendPQExpBuffer(ftStmt, "%s%s IF EXISTS%s",
+ dropStmt, buffer, mark + strlen(buffer));
+ }
+ else
+ appendPQExpBuffer(ftStmt, "%s", dropStmt);
+
+ ahprintf(AH, "%s", ftStmt->data);
+
+ destroyPQExpBuffer(ftStmt);
+ }
+ else
+ ahprintf(AH, "%s", te->dropStmt);
+ }
}
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 458a118..88ea556 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -136,6 +136,7 @@ static int binary_upgrade = 0;
static int disable_dollar_quoting = 0;
static int dump_inserts = 0;
static int column_inserts = 0;
+static int if_exists = 0;
static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
@@ -349,6 +350,7 @@ main(int argc, char **argv)
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &dump_inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
@@ -577,6 +579,9 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
@@ -809,6 +814,7 @@ main(int argc, char **argv)
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
@@ -890,6 +896,7 @@ help(const char *progname)
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 193c1a0..335fdde 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -73,6 +73,7 @@ static int binary_upgrade = 0;
static int column_inserts = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
+static int if_exists = 0;
static int inserts = 0;
static int no_tablespaces = 0;
static int use_setsessauth = 0;
@@ -119,6 +120,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &no_tablespaces, 1},
@@ -352,6 +354,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
appendPQExpBufferStr(pgdumpopts, " --disable-triggers");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (inserts)
appendPQExpBufferStr(pgdumpopts, " --inserts");
if (no_tablespaces)
@@ -564,6 +568,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index bb65253..f732027 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -76,6 +76,7 @@ main(int argc, char **argv)
Archive *AH;
char *inputFileSpec;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int no_data_for_failed_tables = 0;
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
@@ -116,6 +117,7 @@ main(int argc, char **argv)
* the following options don't have an equivalent short option letter
*/
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
@@ -342,6 +344,14 @@ main(int argc, char **argv)
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ if (if_exists && !opts->dropSchema)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+ opts->if_exists = if_exists;
+
if (opts->formatName)
{
switch (opts->formatName[0])
@@ -456,6 +466,7 @@ usage(const char *progname)
printf(_(" -x, --no-privileges skip restoration of access privileges (grant/revoke)\n"));
printf(_(" -1, --single-transaction restore as a single transaction\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
Pavel Stehule escribi�:
It is irony, so this is death code - it is not used now. So I removed it
from patch.Reduced, fixed patch attached + used tests
Nice, thanks.
Here's a new version in which I reworded some comments and docs, and
also inverted the sense of some if/else so that the oneliner case is
first, which makes it more readable IMHO.
However, I don't think this is behaving sanely in pg_dumpall. AFAICT,
pg_dumpall does not pass --clean to pg_dump (in other words it only
emits DROP for the global objects, not the objects contained inside
databases), so passing --if-exists results in failures. Therefore I
think the solution is to not pass --if-exists to pg_dump at all, i.e.
keep it internal to pg_dumpall. But maybe I'm missing something.
I still find the code to inject IF EXISTS to the DROP commands ugly as
sin. I would propose to stop storing the dropStmt in the archive
anymore; instead just store the object identity, which can later be used
to generate both DROP commands, with or without IF EXISTS, and the ALTER
OWNER commands. However, that's a larger project and I don't think we
need to burden this patch with that.
Another point is that we could argue about whether specifying
--if-exists ought to imply --clean instead of erroring out. There's no
backwards compatibility argument to be had; it's not like existing
scripts are going to suddenly start dropping objects that weren't
dropped before.
Other than the pg_dumpall issue, this patch seems ready.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
dump-restore-if-exists-opt-2014-02-28-1.patchtext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 40c69f0..1f0d4de 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -145,7 +145,8 @@ PostgreSQL documentation
<para>
Output commands to clean (drop)
database objects prior to outputting the commands for creating them.
- (Restore might generate some harmless error messages, if any objects
+ (Unless <option>--if-exists</> is also specified,
+ restore might generate some harmless error messages, if any objects
were not present in the destination database.)
</para>
@@ -650,6 +651,17 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ Use conditional commands (i.e. add an <literal>IF EXISTS</literal>
+ clause) when cleaning database objects. This option is not valid
+ unless <option>--clean</> is also specified.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index f337939..fcf5f77 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -301,6 +301,17 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ Use conditional commands (i.e. add an <literal>IF EXISTS</literal>
+ clause) to clean databases and other objects. This option is not valid
+ unless <option>--clean</> is also specified.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--inserts</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index cd60b25..4bc30ce 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -109,7 +109,8 @@
<listitem>
<para>
Clean (drop) database objects before recreating them.
- (This might generate some harmless error messages, if any objects
+ (Unless <option>--if-exists</> is used,
+ this might generate some harmless error messages, if any objects
were not present in the destination database.)
</para>
</listitem>
@@ -490,6 +491,17 @@
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ Use conditional commands (i.e. add an <literal>IF EXISTS</literal>
+ clause) when cleaning database objects. This option is not valid
+ unless <option>--clean</> is also specified.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-data-for-failed-tables</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 6927968..83f7216 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -113,6 +113,7 @@ typedef struct _restoreOptions
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 2b36e45..8395373 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -413,8 +413,76 @@ RestoreArchive(Archive *AHX)
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
- /* Drop it */
- ahprintf(AH, "%s", te->dropStmt);
+
+ /*
+ * Now emit the DROP command, if the object has one. Note we
+ * don't necessarily emit it verbatim; at this point we add an
+ * appropriate IF EXISTS clause, if the user requested it.
+ */
+ if (*te->dropStmt != '\0')
+ {
+ if (!ropt->if_exists)
+ {
+ /* No --if-exists? Then just use the original */
+ ahprintf(AH, "%s", te->dropStmt);
+ }
+ else
+ {
+ char buffer[40];
+ char *mark;
+ char *dropStmt = pg_strdup(te->dropStmt);
+ PQExpBuffer ftStmt = createPQExpBuffer();
+
+ /*
+ * Need to inject IF EXISTS clause after ALTER TABLE
+ * part in ALTER TABLE .. DROP statement
+ */
+ if (strncmp(dropStmt, "ALTER TABLE", 11) == 0)
+ {
+ appendPQExpBuffer(ftStmt,
+ "ALTER TABLE IF EXISTS");
+ dropStmt = dropStmt + 11;
+ }
+
+ /*
+ * ALTER TABLE..ALTER COLUMN..DROP DEFAULT does not
+ * support the IF EXISTS clause, and therefore we
+ * simply emit the original command for such objects.
+ * For other objects, we need to extract the first part
+ * of the DROP which includes the object type. Most of
+ * the time this matches te->desc, so search for that;
+ * however for the different kinds of CONSTRAINTs, we
+ * know to search for hardcoded "DROP CONSTRAINT"
+ * instead.
+ */
+ if (strcmp(te->desc, "DEFAULT") == 0)
+ appendPQExpBuffer(ftStmt, "%s", dropStmt);
+ else
+ {
+ if (strcmp(te->desc, "CONSTRAINT") == 0 ||
+ strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
+ strcmp(te->desc, "FK CONSTRAINT") == 0)
+ strcpy(buffer, "DROP CONSTRAINT");
+ else
+ snprintf(buffer, sizeof(buffer), "DROP %s",
+ te->desc);
+
+ mark = strstr(dropStmt, buffer);
+ Assert(mark != NULL);
+
+ *mark = '\0';
+ appendPQExpBuffer(ftStmt, "%s%s IF EXISTS%s",
+ dropStmt, buffer,
+ mark + strlen(buffer));
+ }
+
+ ahprintf(AH, "%s", ftStmt->data);
+
+ destroyPQExpBuffer(ftStmt);
+
+ pg_free(dropStmt);
+ }
+ }
}
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4fabc1d..80431fb 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -132,6 +132,7 @@ static int binary_upgrade = 0;
static int disable_dollar_quoting = 0;
static int dump_inserts = 0;
static int column_inserts = 0;
+static int if_exists = 0;
static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
@@ -345,6 +346,7 @@ main(int argc, char **argv)
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &dump_inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
@@ -573,6 +575,9 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
@@ -805,6 +810,7 @@ main(int argc, char **argv)
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
@@ -886,6 +892,7 @@ help(const char *progname)
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 193c1a0..335fdde 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -73,6 +73,7 @@ static int binary_upgrade = 0;
static int column_inserts = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
+static int if_exists = 0;
static int inserts = 0;
static int no_tablespaces = 0;
static int use_setsessauth = 0;
@@ -119,6 +120,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &no_tablespaces, 1},
@@ -352,6 +354,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --disable-dollar-quoting");
if (disable_triggers)
appendPQExpBufferStr(pgdumpopts, " --disable-triggers");
+ if (if_exists)
+ appendPQExpBufferStr(pgdumpopts, " --if-exists");
if (inserts)
appendPQExpBufferStr(pgdumpopts, " --inserts");
if (no_tablespaces)
@@ -564,6 +568,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index df9477b..f7f3f51 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -70,6 +70,7 @@ main(int argc, char **argv)
Archive *AH;
char *inputFileSpec;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int no_data_for_failed_tables = 0;
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
@@ -110,6 +111,7 @@ main(int argc, char **argv)
* the following options don't have an equivalent short option letter
*/
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
@@ -336,6 +338,14 @@ main(int argc, char **argv)
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ if (if_exists && !opts->dropSchema)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+ opts->if_exists = if_exists;
+
if (opts->formatName)
{
switch (opts->formatName[0])
@@ -450,6 +460,7 @@ usage(const char *progname)
printf(_(" -x, --no-privileges skip restoration of access privileges (grant/revoke)\n"));
printf(_(" -1, --single-transaction restore as a single transaction\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
2014-02-28 19:31 GMT+01:00 Alvaro Herrera <alvherre@2ndquadrant.com>:
Pavel Stehule escribió:
It is irony, so this is death code - it is not used now. So I removed it
from patch.Reduced, fixed patch attached + used tests
Nice, thanks.
Here's a new version in which I reworded some comments and docs, and
also inverted the sense of some if/else so that the oneliner case is
first, which makes it more readable IMHO.
ok
thank you
However, I don't think this is behaving sanely in pg_dumpall. AFAICT,
pg_dumpall does not pass --clean to pg_dump (in other words it only
emits DROP for the global objects, not the objects contained inside
databases), so passing --if-exists results in failures. Therefore I
think the solution is to not pass --if-exists to pg_dump at all, i.e.
keep it internal to pg_dumpall. But maybe I'm missing something.
I'll look on it tomorrow
I still find the code to inject IF EXISTS to the DROP commands ugly as
sin. I would propose to stop storing the dropStmt in the archive
anymore; instead just store the object identity, which can later be used
to generate both DROP commands, with or without IF EXISTS, and the ALTER
OWNER commands. However, that's a larger project and I don't think we
need to burden this patch with that.
there are more similar parts - and I am not sure if it is little bit heroic
task.
Another point is that we could argue about whether specifying
--if-exists ought to imply --clean instead of erroring out. There's no
backwards compatibility argument to be had; it's not like existing
scripts are going to suddenly start dropping objects that weren't
dropped before.
It is valid idea. I looked on any other options for and I don't known any
similar implication - so I prefer current implementation (no implication).
It is consistent with any other. I have not strong opinion about it - a
user comfort is against a clarity - but two "clean" option can be confusing
maybe.
Regards
Pavel
Show quoted text
Other than the pg_dumpall issue, this patch seems ready.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Hi
However, I don't think this is behaving sanely in pg_dumpall. AFAICT,
pg_dumpall does not pass --clean to pg_dump (in other words it only
emits DROP for the global objects, not the objects contained inside
databases), so passing --if-exists results in failures. Therefore I
think the solution is to not pass --if-exists to pg_dump at all, i.e.
keep it internal to pg_dumpall. But maybe I'm missing something.
I am looking to pg_dumpall code, and I am inclined to don't pass
--if-exists to pg_dump too.
-c, --clean for pg_dumpall means "drop databases"
<<<<<
Usage:
pg_dumpall [OPTION]...
General options:
-f, --file=FILENAME output file name
-V, --version output version information, then exit
--lock-wait-timeout=TIMEOUT fail after waiting TIMEOUT for a table lock
-?, --help show this help, then exit
Options controlling the output content:
-a, --data-only dump only the data, not the schema
-c, --clean clean (drop) databases before recreating
so --if-exists should to mean
DROP DATABASE IF EXISTS dbname
do you agree?
Pavel
2014-02-28 23:13 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
However, I don't think this is behaving sanely in pg_dumpall. AFAICT,
pg_dumpall does not pass --clean to pg_dump (in other words it only
emits DROP for the global objects, not the objects contained inside
databases), so passing --if-exists results in failures. Therefore I
think the solution is to not pass --if-exists to pg_dump at all, i.e.
keep it internal to pg_dumpall. But maybe I'm missing something.I am looking to pg_dumpall code, and I am inclined to don't pass
--if-exists to pg_dump too.-c, --clean for pg_dumpall means "drop databases"
<<<<<
Usage:
pg_dumpall [OPTION]...General options:
-f, --file=FILENAME output file name
-V, --version output version information, then exit
--lock-wait-timeout=TIMEOUT fail after waiting TIMEOUT for a table lock
-?, --help show this help, then exitOptions controlling the output content:
-a, --data-only dump only the data, not the schema
-c, --clean clean (drop) databases before recreatingso --if-exists should to mean
DROP DATABASE IF EXISTS dbname
+ DROP ROLE and DROP TABLESPACE
Show quoted text
do you agree?
Pavel
This patch has redesigned implementation --if-exists for pg_dumpall. Now it
is not propagated to pg_dump, but used on pg_dumpall level.
Regards
Pavel
2014-02-28 23:18 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Show quoted text
2014-02-28 23:13 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
However, I don't think this is behaving sanely in pg_dumpall. AFAICT,
pg_dumpall does not pass --clean to pg_dump (in other words it only
emits DROP for the global objects, not the objects contained inside
databases), so passing --if-exists results in failures. Therefore I
think the solution is to not pass --if-exists to pg_dump at all, i.e.
keep it internal to pg_dumpall. But maybe I'm missing something.I am looking to pg_dumpall code, and I am inclined to don't pass
--if-exists to pg_dump too.-c, --clean for pg_dumpall means "drop databases"
<<<<<
Usage:
pg_dumpall [OPTION]...General options:
-f, --file=FILENAME output file name
-V, --version output version information, then exit
--lock-wait-timeout=TIMEOUT fail after waiting TIMEOUT for a table lock
-?, --help show this help, then exitOptions controlling the output content:
-a, --data-only dump only the data, not the schema
-c, --clean clean (drop) databases before recreatingso --if-exists should to mean
DROP DATABASE IF EXISTS dbname
+ DROP ROLE and DROP TABLESPACE
do you agree?
Pavel
Attachments:
dump-restore-if-exists-opt-2014-02-28-2.patchtext/x-patch; charset=US-ASCII; name=dump-restore-if-exists-opt-2014-02-28-2.patchDownload
commit 3e2e9baa3b5e708b4014b18cbedd2e6b1fc095a5
Author: root <root@nemesis.(none)>
Date: Fri Feb 28 23:31:32 2014 +0100
pg_dumpall --if-exists fix
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 40c69f0..1f0d4de 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -145,7 +145,8 @@ PostgreSQL documentation
<para>
Output commands to clean (drop)
database objects prior to outputting the commands for creating them.
- (Restore might generate some harmless error messages, if any objects
+ (Unless <option>--if-exists</> is also specified,
+ restore might generate some harmless error messages, if any objects
were not present in the destination database.)
</para>
@@ -650,6 +651,17 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ Use conditional commands (i.e. add an <literal>IF EXISTS</literal>
+ clause) when cleaning database objects. This option is not valid
+ unless <option>--clean</> is also specified.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--disable-dollar-quoting</></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index f337939..fcf5f77 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -301,6 +301,17 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ Use conditional commands (i.e. add an <literal>IF EXISTS</literal>
+ clause) to clean databases and other objects. This option is not valid
+ unless <option>--clean</> is also specified.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--inserts</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index cd60b25..4bc30ce 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -109,7 +109,8 @@
<listitem>
<para>
Clean (drop) database objects before recreating them.
- (This might generate some harmless error messages, if any objects
+ (Unless <option>--if-exists</> is used,
+ this might generate some harmless error messages, if any objects
were not present in the destination database.)
</para>
</listitem>
@@ -490,6 +491,17 @@
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ Use conditional commands (i.e. add an <literal>IF EXISTS</literal>
+ clause) when cleaning database objects. This option is not valid
+ unless <option>--clean</> is also specified.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--no-data-for-failed-tables</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 6927968..83f7216 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -113,6 +113,7 @@ typedef struct _restoreOptions
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 2b36e45..8395373 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -413,8 +413,76 @@ RestoreArchive(Archive *AHX)
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
- /* Drop it */
- ahprintf(AH, "%s", te->dropStmt);
+
+ /*
+ * Now emit the DROP command, if the object has one. Note we
+ * don't necessarily emit it verbatim; at this point we add an
+ * appropriate IF EXISTS clause, if the user requested it.
+ */
+ if (*te->dropStmt != '\0')
+ {
+ if (!ropt->if_exists)
+ {
+ /* No --if-exists? Then just use the original */
+ ahprintf(AH, "%s", te->dropStmt);
+ }
+ else
+ {
+ char buffer[40];
+ char *mark;
+ char *dropStmt = pg_strdup(te->dropStmt);
+ PQExpBuffer ftStmt = createPQExpBuffer();
+
+ /*
+ * Need to inject IF EXISTS clause after ALTER TABLE
+ * part in ALTER TABLE .. DROP statement
+ */
+ if (strncmp(dropStmt, "ALTER TABLE", 11) == 0)
+ {
+ appendPQExpBuffer(ftStmt,
+ "ALTER TABLE IF EXISTS");
+ dropStmt = dropStmt + 11;
+ }
+
+ /*
+ * ALTER TABLE..ALTER COLUMN..DROP DEFAULT does not
+ * support the IF EXISTS clause, and therefore we
+ * simply emit the original command for such objects.
+ * For other objects, we need to extract the first part
+ * of the DROP which includes the object type. Most of
+ * the time this matches te->desc, so search for that;
+ * however for the different kinds of CONSTRAINTs, we
+ * know to search for hardcoded "DROP CONSTRAINT"
+ * instead.
+ */
+ if (strcmp(te->desc, "DEFAULT") == 0)
+ appendPQExpBuffer(ftStmt, "%s", dropStmt);
+ else
+ {
+ if (strcmp(te->desc, "CONSTRAINT") == 0 ||
+ strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
+ strcmp(te->desc, "FK CONSTRAINT") == 0)
+ strcpy(buffer, "DROP CONSTRAINT");
+ else
+ snprintf(buffer, sizeof(buffer), "DROP %s",
+ te->desc);
+
+ mark = strstr(dropStmt, buffer);
+ Assert(mark != NULL);
+
+ *mark = '\0';
+ appendPQExpBuffer(ftStmt, "%s%s IF EXISTS%s",
+ dropStmt, buffer,
+ mark + strlen(buffer));
+ }
+
+ ahprintf(AH, "%s", ftStmt->data);
+
+ destroyPQExpBuffer(ftStmt);
+
+ pg_free(dropStmt);
+ }
+ }
}
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4fabc1d..80431fb 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -132,6 +132,7 @@ static int binary_upgrade = 0;
static int disable_dollar_quoting = 0;
static int dump_inserts = 0;
static int column_inserts = 0;
+static int if_exists = 0;
static int no_security_labels = 0;
static int no_synchronized_snapshots = 0;
static int no_unlogged_table_data = 0;
@@ -345,6 +346,7 @@ main(int argc, char **argv)
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"exclude-table-data", required_argument, NULL, 4},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &dump_inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
@@ -573,6 +575,9 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (if_exists && !outputClean)
+ exit_horribly(NULL, "option --if-exists requires -c/--clean option\n");
+
/* Identify archive format to emit */
archiveFormat = parseArchiveFormat(format, &archiveMode);
@@ -805,6 +810,7 @@ main(int argc, char **argv)
ropt->dropSchema = outputClean;
ropt->dataOnly = dataOnly;
ropt->schemaOnly = schemaOnly;
+ ropt->if_exists = if_exists;
ropt->dumpSections = dumpSections;
ropt->aclsSkip = aclsSkip;
ropt->superuser = outputSuperuser;
@@ -886,6 +892,7 @@ help(const char *progname)
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
printf(_(" --exclude-table-data=TABLE do NOT dump data for the named table(s)\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-synchronized-snapshots do not use synchronized snapshots in parallel jobs\n"));
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 193c1a0..7114f1d 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -73,6 +73,7 @@ static int binary_upgrade = 0;
static int column_inserts = 0;
static int disable_dollar_quoting = 0;
static int disable_triggers = 0;
+static int if_exists = 0;
static int inserts = 0;
static int no_tablespaces = 0;
static int use_setsessauth = 0;
@@ -119,6 +120,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &no_tablespaces, 1},
@@ -334,6 +336,13 @@ main(int argc, char *argv[])
exit_nicely(1);
}
+ if (if_exists && !output_clean)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+
if (roles_only && tablespaces_only)
{
fprintf(stderr, _("%s: options -r/--roles-only and -t/--tablespaces-only cannot be used together\n"),
@@ -564,6 +573,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --no-security-labels do not dump security label assignments\n"));
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
@@ -624,7 +634,9 @@ dropRoles(PGconn *conn)
rolename = PQgetvalue(res, i, i_rolname);
- fprintf(OPF, "DROP ROLE %s;\n", fmtId(rolename));
+ fprintf(OPF, "DROP ROLE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ fmtId(rolename));
}
PQclear(res);
@@ -994,7 +1006,9 @@ dropTablespaces(PGconn *conn)
{
char *spcname = PQgetvalue(res, i, 0);
- fprintf(OPF, "DROP TABLESPACE %s;\n", fmtId(spcname));
+ fprintf(OPF, "DROP TABLESPACE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ fmtId(spcname));
}
PQclear(res);
@@ -1148,7 +1162,9 @@ dropDBs(PGconn *conn)
if (strcmp(dbname, "template1") != 0 &&
strcmp(dbname, "postgres") != 0)
{
- fprintf(OPF, "DROP DATABASE %s;\n", fmtId(dbname));
+ fprintf(OPF, "DROP DATABASE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ fmtId(dbname));
}
}
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index df9477b..f7f3f51 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -70,6 +70,7 @@ main(int argc, char **argv)
Archive *AH;
char *inputFileSpec;
static int disable_triggers = 0;
+ static int if_exists = 0;
static int no_data_for_failed_tables = 0;
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
@@ -110,6 +111,7 @@ main(int argc, char **argv)
* the following options don't have an equivalent short option letter
*/
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"if-exists", no_argument, &if_exists, 1},
{"no-data-for-failed-tables", no_argument, &no_data_for_failed_tables, 1},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
@@ -336,6 +338,14 @@ main(int argc, char **argv)
opts->use_setsessauth = use_setsessauth;
opts->no_security_labels = no_security_labels;
+ if (if_exists && !opts->dropSchema)
+ {
+ fprintf(stderr, _("%s: option --if-exists requires -c/--clean option\n"),
+ progname);
+ exit_nicely(1);
+ }
+ opts->if_exists = if_exists;
+
if (opts->formatName)
{
switch (opts->formatName[0])
@@ -450,6 +460,7 @@ usage(const char *progname)
printf(_(" -x, --no-privileges skip restoration of access privileges (grant/revoke)\n"));
printf(_(" -1, --single-transaction restore as a single transaction\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --no-data-for-failed-tables do not restore data of tables that could not be\n"
" created\n"));
printf(_(" --no-security-labels do not restore security labels\n"));
Pavel Stehule escribi�:
This patch has redesigned implementation --if-exists for pg_dumpall. Now it
is not propagated to pg_dump, but used on pg_dumpall level.
Seems sane, thanks.
BTW after this patch, I still don't see an error-free output from
restoring a database on top of itself. One problem is plpgsql, which is
now an extension, so pg_dump emits this error message:
ERROR: cannot drop language plpgsql because extension plpgsql requires it
SUGERENCIA: You can drop extension plpgsql instead.
Another problem is that some DROP commands don't work. For instance, if
the public schema in the target database contains objects that haven't
been dropped yet, the DROP command will fail:
ERROR: cannot drop schema public because other objects depend on it
DETALLE: function bt_metap(text) depends on schema public
function bt_page_items(text,integer) depends on schema public
function bt_page_stats(text,integer) depends on schema public
function f() depends on schema public
function get_raw_page(text,integer) depends on schema public
function heap_page_items(bytea) depends on schema public
function locate_tuple_corruption() depends on schema public
function page_header(bytea) depends on schema public
SUGERENCIA: Use DROP ... CASCADE to drop the dependent objects too.
(The way I got this was by using my 8.2 installation, on which I ran the
regression tests; then I dumped the resulting regression database. The
database on which I restored wasn't clean, as it contained unrelated
junk in the public schema.)
Not sure what's the right answer here to this problem, but it cannot be
attributed to this patch anyway.
I'm about to push this, since other than the above problems, this
functionality seems to be working as designed.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2014-03-03 18:18 GMT+01:00 Alvaro Herrera <alvherre@2ndquadrant.com>:
Pavel Stehule escribió:
This patch has redesigned implementation --if-exists for pg_dumpall. Now
it
is not propagated to pg_dump, but used on pg_dumpall level.
Seems sane, thanks.
BTW after this patch, I still don't see an error-free output from
restoring a database on top of itself. One problem is plpgsql, which is
now an extension, so pg_dump emits this error message:ERROR: cannot drop language plpgsql because extension plpgsql requires it
SUGERENCIA: You can drop extension plpgsql instead.Another problem is that some DROP commands don't work. For instance, if
the public schema in the target database contains objects that haven't
been dropped yet, the DROP command will fail:ERROR: cannot drop schema public because other objects depend on it
DETALLE: function bt_metap(text) depends on schema public
function bt_page_items(text,integer) depends on schema public
function bt_page_stats(text,integer) depends on schema public
function f() depends on schema public
function get_raw_page(text,integer) depends on schema public
function heap_page_items(bytea) depends on schema public
function locate_tuple_corruption() depends on schema public
function page_header(bytea) depends on schema public
SUGERENCIA: Use DROP ... CASCADE to drop the dependent objects too.(The way I got this was by using my 8.2 installation, on which I ran the
regression tests; then I dumped the resulting regression database. The
database on which I restored wasn't clean, as it contained unrelated
junk in the public schema.)
I'll recheck a behave of extensions.
On second hand - usually, preferred way is using a dump related to target
PostgreSQL release
Not sure what's the right answer here to this problem, but it cannot be
attributed to this patch anyway.I'm about to push this, since other than the above problems, this
functionality seems to be working as designed.
Thank you very much
Regards
Pavel
Show quoted text
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
2014-03-04 8:55 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
2014-03-03 18:18 GMT+01:00 Alvaro Herrera <alvherre@2ndquadrant.com>:
Pavel Stehule escribió:
This patch has redesigned implementation --if-exists for pg_dumpall.
Now it
is not propagated to pg_dump, but used on pg_dumpall level.
Seems sane, thanks.
BTW after this patch, I still don't see an error-free output from
restoring a database on top of itself. One problem is plpgsql, which is
now an extension, so pg_dump emits this error message:ERROR: cannot drop language plpgsql because extension plpgsql requires it
SUGERENCIA: You can drop extension plpgsql instead.Another problem is that some DROP commands don't work. For instance, if
the public schema in the target database contains objects that haven't
been dropped yet, the DROP command will fail:ERROR: cannot drop schema public because other objects depend on it
DETALLE: function bt_metap(text) depends on schema public
function bt_page_items(text,integer) depends on schema public
function bt_page_stats(text,integer) depends on schema public
function f() depends on schema public
function get_raw_page(text,integer) depends on schema public
function heap_page_items(bytea) depends on schema public
function locate_tuple_corruption() depends on schema public
function page_header(bytea) depends on schema public
SUGERENCIA: Use DROP ... CASCADE to drop the dependent objects too.(The way I got this was by using my 8.2 installation, on which I ran the
regression tests; then I dumped the resulting regression database. The
database on which I restored wasn't clean, as it contained unrelated
junk in the public schema.)I'll recheck a behave of extensions.
I rechecked extensions and it works - so it can be full quiet when old dump
is imported, but import dump from fresh dumps should to work.
Regards
Pavel
Show quoted text
On second hand - usually, preferred way is using a dump related to target
PostgreSQL releaseNot sure what's the right answer here to this problem, but it cannot be
attributed to this patch anyway.I'm about to push this, since other than the above problems, this
functionality seems to be working as designed.Thank you very much
Regards
Pavel
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services