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

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter_e(at)gmx(dot)net>
Subject: Re: Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
Date: 2013-12-01 20:40:18
Message-ID: CAEZATCVbjDhoTFbKY1+O_Rdt=gs=7GpKVUpKBbZKK0aZCW1agA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On 1 December 2013 07:32, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>
>
> 2013/11/30 Peter Eisentraut <peter_e(at)gmx(dot)net>
>>
>> trailing whitespace
>
>
> fixed,
>

Hi,

I've been looking at this and I think it's mostly in good shape, but I
spotted a few minor issues:

* There's a typo in the notice text in a couple of places --- "does
not exists, skipping" should be "does not exist, skipping".

* In does_not_exist_skipping(), the schema existence checks for
extensions and foreign data wrappers are not necessary, since I don't
think they can be schema-qualified.

* Also in does_not_exist_skipping(), in the block for casts, it is no
longer safe to use format_type_be() because it is now possible for the
types to not exist at this point. So I think it needs to use
TypeNameToString() there instead, otherwise it might raise a no such
type ERROR while trying to issue the NOTICE.

* In DropErrorMsgNonExistent(), I think the ERROR text should report
no such schema in the same way as the NOTICE text when the schema
doesn't exist for consistency with the other ERRORs and NOTICEs.

* Some more code is needed to make DROP OPERATOR FAMILY IF EXISTS
tolerate a non-existent schema.

Attached is an updated patch for those issues. I also tried to tidy up
the code in dropcmds.c a bit, removing some duplicated code, and
making parent_does_not_exist_skipping() have the same signature as
schema_does_not_exist_skipping(). This makes the code in
does_not_exist_skipping() a little neater, and means that
parent_does_not_exist_skipping() can just call
schema_does_not_exist_skipping() to check for the existence of the
parent relation's schema.

I hope those changes are all OK.

Regards,
Dean

Attachment Content-Type Size
drop-if-exists-consistency-v2.patch application/octet-stream 37.4 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Pavel Stehule 2013-12-02 04:55:51 Re: Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
Previous Message Tom Lane 2013-12-01 17:39:30 Re: Update with subselect sometimes returns wrong result

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2013-12-01 20:40:59 Re: review - pg_stat_statements
Previous Message Jeff Janes 2013-12-01 20:40:14 Re: Handling GIN incomplete splits