Fix pg_dump's handling of DROP DATABASE commands in --clean mode.

In commit 4317e0246c645f60c39e6572644cff1cb03b4c65, I accidentally broke
this behavior while rearranging code to ensure that --create wouldn't
affect whether a DATABASE entry gets put into archive-format output.
Thus, 9.2 would issue a DROP DATABASE command in --clean mode, which is
either useless or dangerous depending on the usage scenario.
It should not do that, and no longer does.

A bright spot is that this refactoring makes it easy to allow the
combination of --clean and --create to work sensibly, ie, emit DROP
DATABASE then CREATE DATABASE before reconnecting.  Ordinarily we'd
consider that a feature addition and not back-patch it, but it seems
silly to not include the extra couple of lines required in the 9.2
version of the code.

Per report from Guillaume Lelarge, though this is slightly more extensive
than his proposed patch.

diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 450383083d7e1137c4e8c4604d7091ab4daa457a..a10ae0c6227b7fc7a1659428aed7463ad32d691a 100644 (file)
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -143,7 +143,8 @@ PostgreSQL documentation
        <para>
         Output commands to clean (drop)
         database objects prior to outputting the commands for creating them.
-        (Restore might generate some harmless errors.)
+        (Restore might generate some harmless error messages, if any objects
+        were not present in the destination database.)
        </para>
 
        <para>
@@ -161,8 +162,10 @@ PostgreSQL documentation
        <para>
         Begin the output with a command to create the
         database itself and reconnect to the created database.  (With a
-        script of this form, it doesn't matter which database you connect
-        to before running the script.)
+        script of this form, it doesn't matter which database in the
+        destination installation you connect to before running the script.)
+        If <option>--clean</option> is also specified, the script drops and
+        recreates the target database before reconnecting to it.
        </para>
 
        <para>

diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index b276da6afe6669e75be3b5cd2a105d9cb2ccea55..d4f61669b8828c600623ab05dbf0ffa71a105baf 100644 (file)
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -109,6 +109,8 @@
       <listitem>
        <para>
         Clean (drop) database objects before recreating them.
+        (This might generate some harmless error messages, if any objects
+        were not present in the destination database.)
        </para>
       </listitem>
      </varlistentry>
@@ -118,11 +120,16 @@
       <term><option>--create</option></term>
       <listitem>
        <para>
-        Create the database before restoring into it.  (When this
-        option is used, the database named with <option>-d</option> is
-        used only to issue the initial <command>CREATE DATABASE</>
-        command.  All data is restored into the database name that
-        appears in the archive.)
+        Create the database before restoring into it.
+        If <option>--clean</option> is also specified, drop and
+        recreate the target database before connecting to it.
+       </para>
+
+       <para>
+        When this option is used, the database named with <option>-d</option>
+        is used only to issue the initial <command>DROP DATABASE</> and
+        <command>CREATE DATABASE</> commands.  All data is restored into the
+        database name that appears in the archive.
        </para>
       </listitem>
      </varlistentry>

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 7f47a7c51a3de648444dcf7fbf1466618be90640..1fead285e962db8b4677b633a2b591433bac4b6c 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -303,15 +303,6 @@ RestoreArchive(Archive *AHX)
        /*
         * Check for nonsensical option combinations.
         *
-        * NB: createDB+dropSchema is useless because if you're creating the DB,
-        * there's no need to drop individual items in it.  Moreover, if we tried
-        * to do that then we'd issue the drops in the database initially
-        * connected to, not the one we will create, which is very bad...
-        */
-       if (ropt->createDB && ropt->dropSchema)
-               exit_horribly(modulename, "-C and -c are incompatible options\n");
-
-       /*
         * -C is not compatible with -1, because we can't create a database inside
         * a transaction block.
         */
@@ -456,7 +447,25 @@ RestoreArchive(Archive *AHX)
                {
                        AH->currentTE = te;
 
-                       /* We want anything that's selected and has a dropStmt */
+                       /*
+                        * In createDB mode, issue a DROP *only* for the database as a
+                        * whole.  Issuing drops against anything else would be wrong,
+                        * because at this point we're connected to the wrong database.
+                        * Conversely, if we're not in createDB mode, we'd better not
+                        * issue a DROP against the database at all.
+                        */
+                       if (ropt->createDB)
+                       {
+                               if (strcmp(te->desc, "DATABASE") != 0)
+                                       continue;
+                       }
+                       else
+                       {
+                               if (strcmp(te->desc, "DATABASE") == 0)
+                                       continue;
+                       }
+
+                       /* Otherwise, drop anything that's selected and has a dropStmt */
                        if (((te->reqs & (REQ_SCHEMA | REQ_DATA)) != 0) && te->dropStmt)
                        {
                                ahlog(AH, 1, "dropping %s %s\n", te->desc, te->tag);
@@ -938,9 +947,6 @@ PrintTOCSummary(Archive *AHX, RestoreOptions *ropt)
 
        ahprintf(AH, ";\n;\n; Selected TOC Entries:\n;\n");
 
-       /* We should print DATABASE entries whether or not -C was specified */
-       ropt->createDB = 1;
-
        curSection = SECTION_PRE_DATA;
        for (te = AH->toc->next; te != AH->toc; te = te->next)
        {