Re: BUG #13804: pg_restore returns unexpected error

On Mon, Dec 7, 2015 at 7:06 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

>
>
> On Tue, Dec 8, 2015 at 1:35 AM, <pgdude(at)pgdude(dot)com> wrote:
>
>> pg_restore returns 1 return code indicating failure when it tries to
>> create
>> the public schema using PG 9.4.5
>>
>> Here is my command:
>> pg_restore -h host2 -p 5432 -d postgres -C -c --if-exists -Fd -j 6 -v
>> /path
>> to my dumps/mydump
>>
>> Same error using format: -Fc
>>
>
> If you think this is a bug, could you send a test case? There is not
> enough information regarding what you expect of pg_restore and what it is
> currently doing.
>

​pg_backup_archiver(dot)c(at)3269-3283

​ /*
* 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;
}

This is wrong. The presence of --create causes the schema to still be
present in the newly created database and the logic here (among other
things):

​pg_backup_archiver(dot)c(at)472-488
/*
* 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;
}

​prevents it from being removed from the newly created database.​

​The <if (!ropt->dropSchema)> should be something like <if NOT(dropSchema
AND NOT createDB) {skip public schema}>

I really dislike the negative logic here, though...but it is correct and I
couldn't figure out a better way to write it. Maybe pretty-up and add the
following truth-table to the code comment...

dropSchema createDB skip/makePublic
drop create skip /* present from new database - we are presently "make"
here and that is wrong. */
drop exist make /*!!! dropped from existing database */
leave create skip /* present from new database */
leave exist skip /* ***assumed*** present in old database and not asked
to drop... */

I dislike the reasoning for the last truth table row...but we mustn't drop
the schema as we were not asked to do so. We also cannot conditionally
create the schema since IF NOT EXISTS was only introduced in 9.3...

David J.