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.
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
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. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
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)
============================================================*/
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
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. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
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
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
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
--
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
Show quoted text
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
Attachments:
conditional-drops.patchapplication/octet-stream; name=conditional-drops.patchDownload+130-102
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:
/messages/by-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
--
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/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:
/messages/by-id/507AD08C.5020603@dalibo.comhere is patch, that we use about one year - originally for 9.1 - I did
port to 9.3dropdb 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
Show quoted text
Josh
Attachments:
conditional-drops.patchapplication/octet-stream; name=conditional-drops.patchDownload+183-147
[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
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
2013/3/8 Josh Kupershmidt <schmiddy@gmail.com>:
[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.sqlBut it fails, first at:
...
DROP TRIGGER IF EXISTS tsvectorupdate ON public.test_tsvector;
ERROR: relation "public.test_tsvector" does not existThis 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 existDROP OPERATOR IF EXISTS public.<% (point, widget);
ERROR: type "widget" does not existDROP FUNCTION IF EXISTS public.pt_in_widget(point, widget);
ERROR: type "widget" does not existI'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.
ok
* -- 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.)
no
some people - like we in our company would to use this feature for
quiet and strict mode for plain text format too.
enabling this feature has zero overhead so there are no reason block
it anywhere.
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.
I'll send updated version in next months
Thank you for review
Regards
Pavel
That's all I've had time for so far...
Josh
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Mar 8, 2013 at 2:23 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
2013/3/8 Josh Kupershmidt <schmiddy@gmail.com>:
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.)no
some people - like we in our company would to use this feature for
quiet and strict mode for plain text format too.enabling this feature has zero overhead so there are no reason block
it anywhere.
I'm not sure I understand what you're getting at, but maybe my
proposal wasn't so clear. Right now, you can specify --clean along
with -Fc to pg_dump, and pg_dump will not complain even though this
combination is nonsense. I am proposing that pg_dump error out in this
case, i.e.
$ pg_dump -Fc --file=test.dump --clean -d test
pg_dump: option --clean only valid with plain format dump
Although this lack of an error a (IMO) misfeature of existing pg_dump,
so if you'd rather leave this issue aside for your patch, that is
fine.
Josh
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2013/3/8 Josh Kupershmidt <schmiddy@gmail.com>:
On Fri, Mar 8, 2013 at 2:23 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
2013/3/8 Josh Kupershmidt <schmiddy@gmail.com>:
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.)no
some people - like we in our company would to use this feature for
quiet and strict mode for plain text format too.enabling this feature has zero overhead so there are no reason block
it anywhere.I'm not sure I understand what you're getting at, but maybe my
proposal wasn't so clear. Right now, you can specify --clean along
with -Fc to pg_dump, and pg_dump will not complain even though this
combination is nonsense. I am proposing that pg_dump error out in this
case, i.e.$ pg_dump -Fc --file=test.dump --clean -d test
pg_dump: option --clean only valid with plain format dumpAlthough this lack of an error a (IMO) misfeature of existing pg_dump,
so if you'd rather leave this issue aside for your patch, that is
fine.
I'll see - please, stay tuned to 9.4 first commitfest
Regards
Pavel
Josh
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Mar 8, 2013 at 11:58 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
I'll see - please, stay tuned to 9.4 first commitfest
Hi Pavel,
Just a reminder, I didn't see this patch in the current commitfest. I
would be happy to spend some more time reviewing if you wish to pursue
the patch.
Josh
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2013/6/17 Josh Kupershmidt <schmiddy@gmail.com>:
On Fri, Mar 8, 2013 at 11:58 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
I'll see - please, stay tuned to 9.4 first commitfest
Hi Pavel,
Just a reminder, I didn't see this patch in the current commitfest. I
would be happy to spend some more time reviewing if you wish to pursue
the patch.
Hello
yes, I hadn't free time for finalization of last patch. I hope so I
can do final version next month to next commitfest.
Regards and thank you
Pavel
Josh
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello
2013/3/8 Josh Kupershmidt <schmiddy@gmail.com>:
On Fri, Mar 8, 2013 at 2:23 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
2013/3/8 Josh Kupershmidt <schmiddy@gmail.com>:
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.)no
some people - like we in our company would to use this feature for
quiet and strict mode for plain text format too.enabling this feature has zero overhead so there are no reason block
it anywhere.I'm not sure I understand what you're getting at, but maybe my
proposal wasn't so clear. Right now, you can specify --clean along
with -Fc to pg_dump, and pg_dump will not complain even though this
combination is nonsense. I am proposing that pg_dump error out in this
case, i.e.$ pg_dump -Fc --file=test.dump --clean -d test
pg_dump: option --clean only valid with plain format dumpAlthough this lack of an error a (IMO) misfeature of existing pg_dump,
so if you'd rather leave this issue aside for your patch, that is
fine.
I tested last patch and I am thinking so this patch has sense for
custom format too
[postgres@localhost ~]$ /usr/local/pgsql/bin/pg_dump --clean -t a
-Fc postgres > dump
[postgres@localhost ~]$ psql -c "drop table a"
DROP TABLE
[postgres@localhost ~]$ pg_restore --clean -d postgres dump
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 171; 1259 16462 TABLE
a postgres
pg_restore: [archiver (db)] could not execute query: ERROR: table "a"
does not exist
Command was: DROP TABLE public.a;
WARNING: errors ignored on restore: 1
[postgres@localhost ~]$ /usr/local/pgsql/bin/pg_dump --if-exist
--clean -t a -Fc postgres > dump
[postgres@localhost ~]$ psql -c "drop table a"
DROP TABLE
[postgres@localhost ~]$ pg_restore --clean -d postgres dump
So limit for plain format is not too strict
Regards
Pavel
Josh
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello
2013/3/8 Josh Kupershmidt <schmiddy@gmail.com>:
[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.sqlBut it fails, first at:
...
DROP TRIGGER IF EXISTS tsvectorupdate ON public.test_tsvector;
ERROR: relation "public.test_tsvector" does not existThis 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.
yes, I am thinking so it is probably best solution. Without it I
should to generate a DO statement with necessary conditions. :(
I'll prepare patch and proposal.
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 existDROP OPERATOR IF EXISTS public.<% (point, widget);
ERROR: type "widget" does not existDROP 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.
we can raise a warning instead error ?
Regards
Pavel
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello
remastered patch
still there is a issue with dependencies
Regards
Pavel Stehule
2013/6/17 Josh Kupershmidt <schmiddy@gmail.com>:
Show quoted text
On Fri, Mar 8, 2013 at 11:58 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
I'll see - please, stay tuned to 9.4 first commitfest
Hi Pavel,
Just a reminder, I didn't see this patch in the current commitfest. I
would be happy to spend some more time reviewing if you wish to pursue
the patch.Josh
Attachments:
conditional-drops.patchapplication/octet-stream; name=conditional-drops.patchDownload+142-41
Hello
I am sending a patch that removes strict requirements for DROP IF
EXISTS statements. This behave is similar to our ALTER IF EXISTS
behave now.
postgres=# DROP CAST IF EXISTS (sss AS public.casttesttype);
NOTICE: types "sss" and "public.casttesttype" does not exist, skipping
DROP CAST
postgres=# DROP FUNCTION IF EXISTS public.pt_in_widget(point, widget);
NOTICE: function public.pt_in_widget(point,widget) does not exist, skipping
DROP FUNCTION
postgres=# DROP OPERATOR IF EXISTS public.<% (point, widget);
NOTICE: operator public.<% does not exist, skipping
DROP OPERATOR
postgres=# DROP TRIGGER test_trigger_exists ON no_such_table;
ERROR: relation "no_such_table" does not exist
postgres=# DROP TRIGGER IF EXISTS test_trigger_exists ON no_such_table;
NOTICE: trigger "test_trigger_exists" for table "no_such_table" does
not exist, skipping
DROP TRIGGER
This functionality is necessary for correct quite reload from dump
without possible warnings
Regards
Pavel
2013/7/2 Pavel Stehule <pavel.stehule@gmail.com>:
Show quoted text
Hello
remastered patch
still there is a issue with dependencies
Regards
Pavel Stehule
2013/6/17 Josh Kupershmidt <schmiddy@gmail.com>:
On Fri, Mar 8, 2013 at 11:58 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
I'll see - please, stay tuned to 9.4 first commitfest
Hi Pavel,
Just a reminder, I didn't see this patch in the current commitfest. I
would be happy to spend some more time reviewing if you wish to pursue
the patch.Josh
Attachments:
drop-if-exists-no-double-check.patchapplication/octet-stream; name=drop-if-exists-no-double-check.patchDownload+208-197
On Fri, Jul 5, 2013 at 12:16 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
I am sending a patch that removes strict requirements for DROP IF
EXISTS statements. This behave is similar to our ALTER IF EXISTS
behave now.
+1 for this idea. But this patch should be treated as a separate issue
from the use of IF EXISTS in pg_dump/pg_restore, right? If so, I
suggest starting a new thread about this patch to make reviewing
easier.
Josh
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers