Re: Bug in -c CLI option of pg_dump/pg_restore

Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Oct 18, 2012 at 11:19 AM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>> Hm, but the bug is said to happen only in 9.2, so if we don't backpatch
>> we would leave 9.2 alone exhibiting this behavior.

> Oh, yeah.  I missed that.  But then shouldn't we start by identifying
> which commit broke it before we decide on a fix?

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;

and transferred the responsibility to restore_toc_entry():

+    /*
+     * Ignore DATABASE entry unless we should create it.  We must check this
+     * here, not in _tocEntryRequired, because the createDB option should
+     * not affect emitting a DATABASE entry to an archive file.
+     */
+    if (!ropt->createDB && strcmp(te->desc, "DATABASE") == 0)
+        reqs = 0;

That was a good change for the reason stated in the comment ... but
I missed the fact that the old coding was also suppressing emission of a
DROP DATABASE command in -c mode.  I concur with Guillaume that this is
a bug --- in fact, I got burnt by there being a DROP DATABASE command in
the tar-format special script a few weeks ago, but I supposed that that
was a tar-specific issue of long standing, and didn't pursue it further
at the time.

I think Guillaume's fix is basically OK except it would be better if it
looked more like the code added to restore_toc_entry().  Will adjust and
commit.
        regards, tom lane