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