Thread: BUG #7873: pg_restore --clean tries to drop tables that don't exist
The following bug has been logged on the website: Bug reference: 7873 Logged by: Dave Rolsky Email address: autarch@urth.org PostgreSQL version: 9.2.3 Operating system: Linux Description: = When you pass the --clean option to pg_restore it tries to drop tables without checking if they exist. This results in lots of error output. If you're running pg_restore via an automated process it's very hard to distinguish between these "ok" errors and real errors. It should be using "DROP TABLE IF EXISTS" and the equivalent for constraints.
On Wed, Feb 13, 2013 at 08:22:43PM +0000, autarch@urth.org wrote: > The following bug has been logged on the website: > > Bug reference: 7873 > Logged by: Dave Rolsky > Email address: autarch@urth.org > PostgreSQL version: 9.2.3 > Operating system: Linux > Description: > > When you pass the --clean option to pg_restore it tries to drop tables > without checking if they exist. This results in lots of error output. If > you're running pg_restore via an automated process it's very hard to > distinguish between these "ok" errors and real errors. > > It should be using "DROP TABLE IF EXISTS" and the equivalent for > constraints. Well, I think the question is whether you want error feedback for things that don't exist. I don't really know the answer. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Fri, 15 Feb 2013, Bruce Momjian wrote: > On Wed, Feb 13, 2013 at 08:22:43PM +0000, autarch@urth.org wrote: >> The following bug has been logged on the website: >> >> Bug reference: 7873 >> Logged by: Dave Rolsky >> Email address: autarch@urth.org >> PostgreSQL version: 9.2.3 >> Operating system: Linux >> Description: >> >> When you pass the --clean option to pg_restore it tries to drop tables >> without checking if they exist. This results in lots of error output. If >> you're running pg_restore via an automated process it's very hard to >> distinguish between these "ok" errors and real errors. >> >> It should be using "DROP TABLE IF EXISTS" and the equivalent for >> constraints. > > Well, I think the question is whether you want error feedback for things > that don't exist. I don't really know the answer. Fair enough. It should probably an option to add "if exists", at least. I can't imagine I'm the only using this tool to ship database updates around to different machines, some of which may not have new tables. I'd really like to be able to know when the restore fails versus when it succeeds but is noisy. Cheers, -dave /*============================================================ http://VegGuide.org http://blog.urth.org Your guide to all that's veg House Absolute(ly Pointless) ============================================================*/
On Fri, Feb 15, 2013 at 04:06:12PM -0600, Dave Rolsky wrote: > On Fri, 15 Feb 2013, Bruce Momjian wrote: > > >On Wed, Feb 13, 2013 at 08:22:43PM +0000, autarch@urth.org wrote: > >>The following bug has been logged on the website: > >> > >>Bug reference: 7873 > >>Logged by: Dave Rolsky > >>Email address: autarch@urth.org > >>PostgreSQL version: 9.2.3 > >>Operating system: Linux > >>Description: > >> > >>When you pass the --clean option to pg_restore it tries to drop tables > >>without checking if they exist. This results in lots of error output. If > >>you're running pg_restore via an automated process it's very hard to > >>distinguish between these "ok" errors and real errors. > >> > >>It should be using "DROP TABLE IF EXISTS" and the equivalent for > >>constraints. > > > >Well, I think the question is whether you want error feedback for things > >that don't exist. I don't really know the answer. > > Fair enough. It should probably an option to add "if exists", at > least. I can't imagine I'm the only using this tool to ship database > updates around to different machines, some of which may not have new > tables. I'd really like to be able to know when the restore fails > versus when it succeeds but is noisy. All I can say is I don't remember anyone asking for this in the past. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian <bruce@momjian.us> writes:
> On Fri, Feb 15, 2013 at 04:06:12PM -0600, Dave Rolsky wrote:
>> Fair enough. It should probably an option to add "if exists", at
>> least. I can't imagine I'm the only using this tool to ship database
>> updates around to different machines, some of which may not have new
>> tables. I'd really like to be able to know when the restore fails
>> versus when it succeeds but is noisy.
> All I can say is I don't remember anyone asking for this in the past.
I think it has come up before. I wouldn't object to a pg_dump option to
add IF EXISTS to all the drop commands (though changing the default
behavior would be more controversial). Don't intend to spend my own
time on it though ...
regards, tom lane
2013/2/16 Tom Lane <tgl@sss.pgh.pa.us>: > Bruce Momjian <bruce@momjian.us> writes: >> On Fri, Feb 15, 2013 at 04:06:12PM -0600, Dave Rolsky wrote: >>> Fair enough. It should probably an option to add "if exists", at >>> least. I can't imagine I'm the only using this tool to ship database >>> updates around to different machines, some of which may not have new >>> tables. I'd really like to be able to know when the restore fails >>> versus when it succeeds but is noisy. > >> All I can say is I don't remember anyone asking for this in the past. > > I think it has come up before. I wouldn't object to a pg_dump option to > add IF EXISTS to all the drop commands (though changing the default > behavior would be more controversial). Don't intend to spend my own > time on it though ... we use this feature more than one year. I'll send patch at Monday Regards Pavel Stehule > > regards, tom lane > > > -- > Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-bugs
Hello 2013/2/16 Pavel Stehule <pavel.stehule@gmail.com>: > 2013/2/16 Tom Lane <tgl@sss.pgh.pa.us>: >> Bruce Momjian <bruce@momjian.us> writes: >>> On Fri, Feb 15, 2013 at 04:06:12PM -0600, Dave Rolsky wrote: >>>> Fair enough. It should probably an option to add "if exists", at >>>> least. I can't imagine I'm the only using this tool to ship database >>>> updates around to different machines, some of which may not have new >>>> tables. I'd really like to be able to know when the restore fails >>>> versus when it succeeds but is noisy. >> >>> All I can say is I don't remember anyone asking for this in the past. >> >> I think it has come up before. I wouldn't object to a pg_dump option to >> add IF EXISTS to all the drop commands (though changing the default >> behavior would be more controversial). Don't intend to spend my own >> time on it though ... > > we use this feature more than one year. > > I'll send patch at Monday here is patch, that we use about one year - originally for 9.1 - I did port to 9.3 Regards Pavel > > Regards > > Pavel Stehule > > > >> >> regards, tom lane >> >> >> -- >> Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) >> To make changes to your subscription: >> http://www.postgresql.org/mailpref/pgsql-bugs
Attachment
On Tue, Feb 19, 2013 at 6:00 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > 2013/2/16 Pavel Stehule <pavel.stehule@gmail.com>: >> 2013/2/16 Tom Lane <tgl@sss.pgh.pa.us>: >>> I think it has come up before. I wouldn't object to a pg_dump option to >>> add IF EXISTS to all the drop commands (though changing the default >>> behavior would be more controversial). Don't intend to spend my own >>> time on it though ... FYI, it was proposed here: http://www.postgresql.org/message-id/507AD08C.5020603@dalibo.com > here is patch, that we use about one year - originally for 9.1 - I did > port to 9.3 dropdb and dropuser both support a similar option named --if-exists. I suggest --if-exists instead of --conditional-drops for consistency. I've only glanced at the patch, but if it makes no sense to use --conditional-drops (or --if-exists, whatever it ends up being called) without --clean, then attempting to do so should raise an error. Josh
Hello 2013/2/21 Josh Kupershmidt <schmiddy@gmail.com>: > On Tue, Feb 19, 2013 at 6:00 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> 2013/2/16 Pavel Stehule <pavel.stehule@gmail.com>: >>> 2013/2/16 Tom Lane <tgl@sss.pgh.pa.us>: >>>> I think it has come up before. I wouldn't object to a pg_dump option to >>>> add IF EXISTS to all the drop commands (though changing the default >>>> behavior would be more controversial). Don't intend to spend my own >>>> time on it though ... > > FYI, it was proposed here: > http://www.postgresql.org/message-id/507AD08C.5020603@dalibo.com > >> here is patch, that we use about one year - originally for 9.1 - I did >> port to 9.3 > > dropdb and dropuser both support a similar option named --if-exists. I > suggest --if-exists instead of --conditional-drops for consistency. > I've only glanced at the patch, but if it makes no sense to use > --conditional-drops (or --if-exists, whatever it ends up being called) > without --clean, then attempting to do so should raise an error. so * --conditional-drops replaced by --if-exists * -- additional check, available only with -c option * fix bug with dump custom functions Regards Pavel > > Josh
Attachment
[Moving to -hackers]
On Sat, Feb 23, 2013 at 2:51 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> so
>
> * --conditional-drops replaced by --if-exists
Thanks for the fixes, I played around with the patch a bit. I was sort
of expecting this example to work (after setting up the regression
database with `make installcheck`)
pg_dump --clean --if-exists -Fp -d regression --file=regression.sql createdb test psql -v ON_ERROR_STOP=1
--single-transaction-d test -f regression.sql
But it fails, first at: ... DROP TRIGGER IF EXISTS tsvectorupdate ON public.test_tsvector; ERROR: relation
"public.test_tsvector"does not exist
This seems like a shortcoming of DROP TRIGGER ... IF EXISTS, and it
looks like DROP RULE ... IF EXISTS has the same problem. I recall DROP
... IF EXISTS being fixed recently for not to error out if the schema
specified for the object does not exist, and ISTM the same arguments
could be made in favor of fixing DROP TRIGGER/TABLE ... IF EXISTS not
to error out if the table doesn't exist.
Working further through the dump of the regression database, these
also present problems for --clean --if-exists dumps:
DROP CAST IF EXISTS (text AS public.casttesttype); ERROR: type "public.casttesttype" does not exist
DROP OPERATOR IF EXISTS public.<% (point, widget); ERROR: type "widget" does not exist
DROP FUNCTION IF EXISTS public.pt_in_widget(point, widget); ERROR: type "widget" does not exist
I'm not sure whether DROP CAST/OPERATOR/FUNCTION IF EXISTS should be
more tolerant of nonexistent types, of if the mess could perhaps be
avoided by dump reordering.
Note, this usability problem affects unpatched head as well:
pg_dump -Fc -d regression --file=regression.dump pg_restore --clean -1 -d regression regression.dump ... pg_restore:
[archiver(db)] could not execute query: ERROR: type
"widget" does not exist Command was: DROP FUNCTION public.widget_out(widget);
(The use here is a little different than the first example above, but
I would still expect this case to work.) The above problems with IF
EXISTS aren't really a problem of the patch per se, but IMO it would
be nice to straighten all the issues out together for 9.4.
> * -- additional check, available only with -c option
Cool. I think it would also be useful to check that --clean may only
be used with --format=p to avoid any confusion there. (This issue
could be addressed in a separate patch if you'd rather not lard this
patch.)
Some comments on the changes:
1. There is at least one IF EXISTS check missing from pg_dump.c, see
for example this statement from a dump of the regression database with
--if-exists:
ALTER TABLE public.nv_child_2010 DROP CONSTRAINT nv_child_2010_d_check;
2. Shouldn't pg_restore get --if-exists as well?
3.
+ printf(_(" --if-exists don't report error if
cleaned object doesn't exist\n"));
This help output bleeds just over our de facto 80-character limit.
Also contractions seem to be avoided elsewhere. It's a little hard to
squeeze a decent explanation into one line, but perhaps:
Use IF EXISTS when dropping objects
would be better. The sgml changes could use some wordsmithing and
grammar fixes. I could clean these up for you in a later version if
you'd like.
4. There seem to be spurious whitespace changes to the function
prototype and declaration for _printTocEntry.
That's all I've had time for so far...
Josh