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
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"));
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
