Re: Fwd: [BUGS] BUG #14247: COMMENT is restored on wrong database

Поиск
Список
Период
Сортировка
От David G. Johnston
Тема Re: Fwd: [BUGS] BUG #14247: COMMENT is restored on wrong database
Дата
Msg-id CAKFQuwZQpk-JW3kjnJx06fXHiGobRzeu=r3sTW1yJ+LhfhCPdg@mail.gmail.com
обсуждение исходный текст
Ответы Re: Fwd: [BUGS] BUG #14247: COMMENT is restored on wrong database  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers
Дерево обсуждения
Moving to -hackers since this is getting into details

Bug Report # 14247

On Tue, Aug 2, 2016 at 4:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Do you have an opinion on this following?

I think the real problem in this area is that the division of labor
we have between pg_dump and pg_dumpall isn't very good for real-world
use cases.

​I won't disagree here​.
 
  I'm not at all sure what a better answer would look like.
But I'd rather see us take two steps back and consider the issue
holistically instead of trying to band-aid individual misbehaviors.

The fact that pg_dump is emitting COMMENT ON DATABASE at all is
fundamentally wrong given the existing division-of-labor decisions,
namely that pg_dump is responsible for objects within a database
not for database-level properties.

​But I have to disagree with this in the presence of the --create flag.


It's entirely possible that some feature like COMMENT ON CURRENT DATABASE
would be a necessary component of a holistic solution,
​ [ various other ON CURRENT commands elidded ]​

In reading the code for the ​public schema bug I never fully got my head around how dump/restore interacts with multiple databases.  Specifically, in RestoreArchive, why we basically continue instead of breaking once we've encountered the "DATABASE" entry and decide that we need to drop the DB due to (createDB && dropSchema).

​OTOH, seeing the code for the public schema exception I'm inclined to think ​that adding something like the follow just after the public schema block just patched:

/* If the user didn't request that a new database be created skip emitting
 * commands targeting the current database as they may not have the same
 * name as the database being restored into.
 * 
 * XXX This too is ugly but the treatment of globals is not presently amenable to pretty solutions
 */
if (!ropt->createDB))
{
if (strcmp(te->desc, "DATABASE") != 0  //this is quite possibly too broad a check...I'm always concerned about false positives in cases like these.
                         return;
}

​Though reading it now that seems a bit like throwing out the baby with the bath water; but then again if they are not requesting a database be created and they happen to have a database of the same name already in place it would be unexpected that it would not have been properly configured.

Assuming I'm not missing something here it seems like an easy and internally consistent hack/fix for a rare but real concern.  However, since the existing code doesn't provoke an error it doesn't seem like something this should back-patched (I'm not convinced either way though).

David J.

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: Why we lost Uber as a user
Следующее
От: Alvaro Herrera
Дата:
Сообщение: Re: Increasing timeout of poll_query_until for TAP tests