Re: Further cleanup of pg_dump/pg_restore item selection code

2018-01-24 Thread Tom Lane
"David G. Johnston"  writes:
> On Wednesday, January 24, 2018, Tom Lane  wrote:
>> The same behaviors occur if you do e.g.
>>  pg_dump -Fc -t sometable somedb | pg_restore --create
>> which is another undocumented misbehavior: the docs for pg_restore do not
>> suggest that --create might fail if the source archive was selective.

> pg_restore -t:

> "When -t is specified, pg_restore makes no attempt to restore any other
> database objects that the selected table(s) might depend upon. Therefore,
> there is no guarantee that a specific-table restore into a clean database
> will succeed."

I think you might be missing one of the main points here, which is
that --create is specified as causing both a CREATE DATABASE and a
reconnect to that database.  If it silently becomes a no-op, which
is what happens today, the restore is likely to go into the wrong
database entirely (or at least not the DB the user expected).

I won't deny the possibility that some people are depending on the
existing wrong behavior, but that's a hazard with any bug fix.

regards, tom lane



Re: Further cleanup of pg_dump/pg_restore item selection code

2018-01-24 Thread David G. Johnston
On Wednesday, January 24, 2018, Tom Lane  wrote:
>
>
> This does not go all the way towards making pg_restore's item selection
> switches dump subsidiary objects the same as pg_dump would, because
> pg_restore doesn't really have enough info to deal with indexes and
> table constraints the way pg_dump does.  Perhaps we could intuit the
> parent table by examining object dependencies, but that would be a
> bunch of new logic that seems like fit material for a different patch.
> In the meantime I just added a disclaimer to pg_restore.sgml explaining
> this.
>

Unless we really wanted to keep prior-version compatibility it would seem
more reliable to have pg_dump generate a new TOC entry type describing
these dependencies and have pg_restore read and interpret them during
selective restore mode.  Basically a cross-referencing "if you restore A
also restore B". Where A can only be tables and B indexes and constraints
(if we can get away with it being that limited initially).

David J.


Re: Further cleanup of pg_dump/pg_restore item selection code

2018-01-24 Thread David G. Johnston
On Wednesday, January 24, 2018, Tom Lane  wrote:
>
> I think you might be missing one of the main points here, which is
> that --create is specified as causing both a CREATE DATABASE and a
> reconnect to that database.
>

I missed the implication though I read and even thought about questioning
that aspect (in pg_dump though, not restore).

Should pg_restore fail if asked to --create without a database entry in the
TOC?

David J.


Re: Further cleanup of pg_dump/pg_restore item selection code

2018-01-25 Thread Tom Lane
"David G. Johnston"  writes:
> Should pg_restore fail if asked to --create without a database entry in the
> TOC?

Yeah, I wondered about that too.  This patch makes it a non-issue for
archives created with v11 or later pg_dump, but there's still a hazard
if you're restoring from an archive made by an older pg_dump.  Is it
worth putting in logic to catch that?

Given the lack of field complaints, I felt fixing it going forward is
sufficient, but there's room to argue differently.

regards, tom lane



Re: Further cleanup of pg_dump/pg_restore item selection code

2018-01-24 Thread Tom Lane
"David G. Johnston"  writes:
> On Wednesday, January 24, 2018, Tom Lane  wrote:
>> The same behaviors occur if you do e.g.
>>  pg_dump -Fc -t sometable somedb | pg_restore --create
>> which is another undocumented misbehavior: the docs for pg_restore do not
>> suggest that --create might fail if the source archive was selective.

> pg_restore -t:

> "When -t is specified, pg_restore makes no attempt to restore any other
> database objects that the selected table(s) might depend upon. Therefore,
> there is no guarantee that a specific-table restore into a clean database
> will succeed."

I think you might be missing one of the main points here, which is
that --create is specified as causing both a CREATE DATABASE and a
reconnect to that database.  If it silently becomes a no-op, which
is what happens today, the restore is likely to go into the wrong
database entirely (or at least not the DB the user expected).

I won't deny the possibility that some people are depending on the
existing wrong behavior, but that's a hazard with any bug fix.

regards, tom lane



Re: Further cleanup of pg_dump/pg_restore item selection code

2018-01-24 Thread David G. Johnston
On Wednesday, January 24, 2018, Tom Lane  wrote:
>
>
> This does not go all the way towards making pg_restore's item selection
> switches dump subsidiary objects the same as pg_dump would, because
> pg_restore doesn't really have enough info to deal with indexes and
> table constraints the way pg_dump does.  Perhaps we could intuit the
> parent table by examining object dependencies, but that would be a
> bunch of new logic that seems like fit material for a different patch.
> In the meantime I just added a disclaimer to pg_restore.sgml explaining
> this.
>

Unless we really wanted to keep prior-version compatibility it would seem
more reliable to have pg_dump generate a new TOC entry type describing
these dependencies and have pg_restore read and interpret them during
selective restore mode.  Basically a cross-referencing "if you restore A
also restore B". Where A can only be tables and B indexes and constraints
(if we can get away with it being that limited initially).

David J.


Re: Further cleanup of pg_dump/pg_restore item selection code

2018-01-24 Thread David G. Johnston
On Wednesday, January 24, 2018, Tom Lane  wrote:
>
> I think you might be missing one of the main points here, which is
> that --create is specified as causing both a CREATE DATABASE and a
> reconnect to that database.
>

I missed the implication though I read and even thought about questioning
that aspect (in pg_dump though, not restore).

Should pg_restore fail if asked to --create without a database entry in the
TOC?

David J.


Re: Further cleanup of pg_dump/pg_restore item selection code

2018-01-25 Thread Tom Lane
"David G. Johnston"  writes:
> Should pg_restore fail if asked to --create without a database entry in the
> TOC?

Yeah, I wondered about that too.  This patch makes it a non-issue for
archives created with v11 or later pg_dump, but there's still a hazard
if you're restoring from an archive made by an older pg_dump.  Is it
worth putting in logic to catch that?

Given the lack of field complaints, I felt fixing it going forward is
sufficient, but there's room to argue differently.

regards, tom lane