Re: Further cleanup of pg_dump/pg_restore item selection code
"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
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
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" 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" 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
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
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" 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