On 1 December 2013 07:32, Pavel Stehule <pavel.stehule@gmail.com> wrote: > > > > 2013/11/30 Peter Eisentraut <peter_e@gmx.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.