Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist

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

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

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