[BUGS] BUG #14271: Please fix 13804 bug

[a***@gmail.com]

The following bug has been logged on the website:

Bug reference: 14271
Logged by: Anton Dyachenko
Email address: ***@gmail.com
PostgreSQL version: 9.5.3
Operating system: Windows
Description:

The issue of this bug in the output of dump.

Current output:
line 40: CREATE SCHEMA public;

Fix of the problem:
CREATE SCHEMA IF NOT EXISTS public;

If there is some problem to change dump generator to do this please do this
at least for --if-exist parameter.

[Michael Paquier]

Post by a***@gmail.com
line 40: CREATE SCHEMA public;
CREATE SCHEMA IF NOT EXISTS public;
If there is some problem to change dump generator to do this please do this
at least for --if-exist parameter.

Could you describe a little bit more in details the command of pg_dump
that you are using? It would be also good to get an exact idea of the
dump you are seeing, what you think it should do with a detailed
example, and what are the objects on your database at the moment of
taking the dump.

[David G. Johnston]

Post by Michael Paquier

Post by a***@gmail.com
line 40: CREATE SCHEMA public;
CREATE SCHEMA IF NOT EXISTS public;
If there is some problem to change dump generator to do this please do

this

Post by a***@gmail.com
at least for --if-exist parameter.

Could you describe a little bit more in details the command of pg_dump
that you are using? It would be also good to get an exact idea of the
dump you are seeing, what you think it should do with a detailed
example, and what are the objects on your database at the moment of
taking the dump.

See also...

https://www.postgresql.org/message-id/20160312230919.GA11080%40hermes.hilbert.loc

I'd say there is room for both doc and code improvement here. Not exactly
sure what.

David J.

[Tom Lane]

Post by Michael Paquier

Post by a***@gmail.com
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

[David G. Johnston]

Post by Tom Lane
So it looks to me like an appropriate fix would be basically this
- 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.

I concurred on the original thread bug report thread.

https://www.postgresql.org/message-id/CAKFQuwbYXU-z0uxnLWV_1awd16JiMnJnKnMQgYrUDJT4uqmmjQ%40mail.gmail.com

David J.

[Tom Lane]

Post by David G. Johnston

Post by Tom Lane
So it looks to me like an appropriate fix would be basically this
- if (!ropt->dropSchema)
+ if (!(ropt->dropSchema && !ropt->createDB))

I concurred on the original thread bug report thread.
https://www.postgresql.org/message-id/CAKFQuwbYXU-z0uxnLWV_1awd16JiMnJnKnMQgYrUDJT4uqmmjQ%40mail.gmail.com

Heh ... that's what I get for not bothering to go read the other thread.
Clearly we dropped the ball on pushing that report through to a fix.

regards, tom lane