Re: Bug in -c CLI option of pg_dump/pg_restore - Mailing list pgsql-hackers

I wrote:
> It looks like I broke this in commit
> 4317e0246c645f60c39e6572644cff1cb03b4c65, because I removed this from
> _tocEntryRequired():

> -    /* Ignore DATABASE entry unless we should create it */
> -    if (!ropt->createDB && strcmp(te->desc, "DATABASE") == 0)
> -        return 0;

Actually, on closer look, this change provides the foundation needed to
do more than just fix this bug.  We can also make the combination
"pg_dump -C -c" work sanely, which it never has before.  I propose that
we fix this with the attached patch (plus probably some documentation
changes, though I've not looked yet to see what the docs say about it).
With this fix, the output for "-C -c" looks like

DROP DATABASE regression;
CREATE DATABASE regression WITH ...
ALTER DATABASE regression OWNER ...
\connect regression
... etc ...

which seems to me to be just about exactly what one would expect.

The patch also gets rid of a kluge in PrintTOCSummary, which was needed
because of the old coding in _tocEntryRequired(), but no longer is.

            regards, tom lane

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 7f47a7c..1fead28 100644
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** RestoreArchive(Archive *AHX)
*** 303,317 ****
      /*
       * 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.
       */
--- 303,308 ----
*************** RestoreArchive(Archive *AHX)
*** 456,462 ****
          {
              AH->currentTE = te;

!             /* We want 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);
--- 447,471 ----
          {
              AH->currentTE = te;

!             /*
!              * 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);
*************** PrintTOCSummary(Archive *AHX, RestoreOpt
*** 938,946 ****

      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)
      {
--- 947,952 ----