From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Further cleanup of pg_dump/pg_restore item selection code |
Date: | 2018-01-25 02:49:37 |
Message-ID: | 32668.1516848577@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
As I've been hacking on the pg_dump code recently, I got annoyed at the
ugliness of its code for deciding whether or not to emit database-related
TOC entries. That logic is implemented in a completely different place
from other TOC entry selection decisions, 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
will not emit a CREATE DATABASE command as you might expect, because
the use of -t prevents the DATABASE TOC entry from being made in the
first place. Also,
pg_dump --clean --create -t sometable somedb
will not emit any DROP commands: the code expects that with the
combination of --clean and --create, it should emit only DROP
DATABASE, but nothing happens because there's no DATABASE entry.
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.
Another set of problems is that if you use pg_restore's item
selection switches (-t etc), those do not restore ACLs, comments,
or security labels associated with the selected object(s). This
is strange, and unlike the behavior of the comparable switches in
pg_dump, and certainly undocumented.
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. And it fixes selective pg_restore to include
subsidiary ACLs, comments, and security labels.
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.
I also made an effort to make _tocEntryRequired() a little better
organized and better documented. It's grown a lot of different
behaviors over the years without much thought about layout or
keeping related checks together. (There's a chunk in the middle
of the function that now needs to be indented one stop to the right,
but in the interests of keeping the patch reviewable, I've not done
so yet.)
Comments?
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
pg_restore-item-selection-improvements-1.patch | text/x-diff | 13.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2018-01-25 03:12:49 | Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump |
Previous Message | Jing Wang | 2018-01-25 02:15:09 | Re: [HACKERS] Support to COMMENT ON DATABASE CURRENT_DATABASE |