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

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, Oct 18, 2012 at 11:19 AM, Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)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