Re: recent ALTER whatever .. SET SCHEMA refactoring

On Mon, Jan 7, 2013 at 2:14 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Kohei KaiGai escribió:
>
>> Function and collation are candidates of this special case handling;
>> here are just two kinds of object.
>>
>> Another idea is to add a function-pointer as argument of
>> AlterNamespace_internal for (upcoming) object classes that takes
>> special handling for detection of name collision.
>> My personal preference is the later one, rather than hardwired
>> special case handling.
>> However, it may be too elaborate to handle just two exceptions.
>
> I think this idea is fine.  Pass a function pointer which is only
> not-NULL for the two exceptional cases; the code should have an Assert
> that either the function pointer is passed, or there is a nameCacheId to
> use.  That way, the object types we already handle in the simpler way do
> not get any more complicated than they are today, and we're not forced
> to create useless callbacks for objects were the lookup is trivial.  The
> function pointer should return boolean, true when the function/collation
> is already in the given schema; that way, the message wording is only
> present in AlterObjectNamespace_internal.

It seems overly complex to me.  What's wrong with putting special-case
logic directly into the function?  That seems cleaner and easier to
understand, and there's no real downside AFAICS.  We have similar
special cases elsewhere; the code can't be simpler than the actual
logic.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company