Re: BUG #14271: Please fix 13804 bug

Michael Paquier <michael.paquier@gmail.com> writes:
> On Mon, Aug 1, 2016 at 11:19 AM,  <amdjachenko@gmail.com> wrote:
>> Current output:
>> line 40: CREATE SCHEMA public;

> Could you describe a little bit more in details the command of pg_dump
> that you are using?

I experimented with this a bit and found that you only get that output
if you use -C and -c together.  If you use -c alone, you do get a
"CREATE SCHEMA public", but first you get "DROP SCHEMA public",
so that seems okay.  (I suppose it could be problematic if you don't
have permissions to do that drop, but that would be a different
complaint eh?)

After poking around awhile, it seems like the real problem is with this
logic in _printTocEntry() that special-cases the public schema:

    /*
     * Avoid dumping the public schema, as it will already be created ...
     * unless we are using --clean mode, in which case it's been deleted and
     * we'd better recreate it.  Likewise for its comment, if any.
     */
    if (!ropt->dropSchema)
    {
        if (strcmp(te->desc, "SCHEMA") == 0 &&
            strcmp(te->tag, "public") == 0)
            return;
        /* The comment restore would require super-user privs, so avoid it. */
        if (strcmp(te->desc, "COMMENT") == 0 &&
            strcmp(te->tag, "SCHEMA public") == 0)
            return;
    }

That seems okay on its face, but the problem is that it has too simplistic
a notion of what --clean mode means.  In particular, if we consult
RestoreArchive() to find out what dropSchema *really* does:

    /*
     * Drop the items at the start, in reverse order
     */
    if (ropt->dropSchema)
    {
        for (te = AH->toc->prev; te != AH->toc; te = te->prev)
        {
            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 */

In other words, the combination of -C and -c is supposed to issue
"DROP DATABASE currentdb" then "CREATE DATABASE currentdb", but not
individual drops against objects contained in the DB.  Therefore, we
should only expect that the public schema needs to be created if we are
in -c node and NOT in -C mode.  If both are set then we're working in a
virgin database that should be expected to contain a public schema.

So it looks to me like an appropriate fix would be basically this
in _printTocEntry():

-    if (!ropt->dropSchema)
+    if (!(ropt->dropSchema && !ropt->createDB))

plus some suitable adjustment of the comment.  I'm too lazy/tired
to test this theory right now, though.

            regards, tom lane