Hello

2014/1/16 Jeevan Chalke <jeevan.cha...@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.
> +       }
> +
> +       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.
>
>
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




> Marking "Waiting on author".
>
> Thanks
>
>
> --
> Jeevan B Chalke
> Principal Software Engineer, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>
commit 3fecb7805261e96db764fffcae2bb912538903f5
Author: Pavel Stehule <pavel.steh...@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"));
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to