Re: recent ALTER whatever .. SET SCHEMA refactoring

On Mon, Jan 7, 2013 at 2:14 PM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)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