Re: ALTER command reworks

Hi Dimitri,

Thanks for your checks.

2012/11/19 Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>:
> Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> writes:
>> The attached patch is the revised version of ALTER RENAME TO
>> consolidation. According to the previous suggestion, it uses
>> a common logic to check object-naming duplication at
>> check_duplicate_objectname().
>
> I think you need to better comment which object types are supported by
> the new generic function and which are not in AlterObjectRename().
>
OK, Are you suggesting to add a "generic" comments such as "Generic
function to change the name of a given object, for simple cases ...",
not a list of OBJECT_* at the head of this function, aren't you?

>> At the code path on AlterObjectNamespace_internal, it lost
>> ObjectType information to the supplied object, so I also added
>> get_object_type() function at objectaddress.c for inverse
>> translation from a couple of classId/objectId to OBJECT_* label.
>
> I don't understand that part, and it looks like it would be way better
> to find a way to pass down the information you already have earlier in
> the code than to compute it again doing syscache lookups…
>
The pain point is AlterObjectNamespace_internal is not invoked by
only ExecAlterObjectSchemaStmt(), but AlterObjectNamespace_oid()
also.
It is the reason why I had to put get_object_type() to find out OBJECT_*
from the supplied ObjectAddress, because this code path does not
available to pass down its ObjectType from the caller.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>