Re: Further cleanup of pg_dump/pg_restore item selection code

[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

[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

[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

[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

Further cleanup of pg_dump/pg_restore item selection code

[David G. Johnston]

On Wednesday, January 24, 2018, Tom Lane  wrote:

> and it has a bunch of strange
> behaviors that can really only be characterized as bugs.  An example is
> that
>
> pg_dump --create -t sometable somedb
>
>
pg_dump -t:

"The -n and -N switches have no effect when -t is used, because tables
selected by -t will be dumped regardless of those switches, and non-table
objects will not be dumped."

a database is a non-table object...though the proposed behavior seems
reasonable; but maybe worthy of being pointed out just like the -n switches
are.

(probably been asked before but why 'no effect' instead of 'will cause the
dump to fail before it begins'?)


> 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."

That -t was specified on the dump instead of the restore could be clarified
but applies nonetheless.

Do we document anywhere what is a "database object" and what are "property
objects" that are restored even when -t is specified?


> Attached is a patch that proposes to clean this up.  It arranges
> things so that CREATE DATABASE (and, if --clean, DROP DATABASE)
> is emitted if and only if you said --create, regardless of other
> selectivity switches.


Makes sense, the bit about regardless of other switches probably should
find it's way into the docs.

Adding a drop database where there never was one before is a bit
disconcerting though...even if the switches imply the user should be
expecting one,


>
>  And it fixes selective pg_restore to include
> subsidiary ACLs, comments, and security labels.


+1

David J.