Re: ALTER command reworks

2012/10/5 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> Kohei KaiGai escribió:
>> 2012/8/31 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
>> > 2012/8/30 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> >> On Mon, Aug 13, 2012 at 3:35 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>
>> >>> Was it a right decision to track dependency of large object using
>> >>> oid of pg_largeobject, instead of pg_largeobject_metadata?
>> >>> IIRC, the reason why we used oid of pg_largeobject is backward
>> >>> compatibility for applications that tries to reference pg_depend
>> >>> with built-in oids.
>> >>
>> >> I think it was a terrible decision and I'm pretty sure I said as much
>> >> at the time, but I lost the argument, so I'm inclined to think we're
>> >> stuck with continuing to support that kludge.
>> >>
>> > OK, we will keep to implement carefully...
>
> After reviewing this patch, I think we need to revisit this decision.
>
>> > OK, I'll split the patch into three (isn't it?) portions; RENAME, SET OWNER
>> > and SET SCHEMA, with all above your suggestions.
>> >
>> As attached, I split off the original one into three portions; for set-schema,
>> set-owner and rename-to. Please apply them in order of patch filename.
>> Regarding to the error message, RenameErrorMsgAlreadyExists() was added
>> to handle per object type messages in case when aclcheck_error() is not
>> available to utilize.
>
> Here's the remaining piece; the RENAME part. I have reworked it a bit,
> but it needs a bit more work yet. Note this:
>
> alvherre=# alter function foo.g(int, int) rename to g;
> ERROR: function foo.g(integer,integer) already exists in schema "foo"
>
> The previous code would have said
>
> alvherre=# alter function foo.g(int, int) rename to g;
> ERROR: function g(integer, integer) already exists in schema "foo"
>
> Note that the function name is not qualified here. The difference is
> that the original code was calling funcname_signature_string() to format
> the function name, and the new code is calling format_procedure(). This
> seems wrong to me; please rework.
>
> I haven't checked other object renames, but I think they should be okay
> because functions are the only objects for which we pass the OID instead
> of the name to the error message function.
>
The attached patch fixes the messaging issue.
I newly add func_signature_string_oid() that returns compatible function's
signature, but takes its object-id.

So, the error message is now constructed as:
+ case OBJECT_AGGREGATE:
+ case OBJECT_FUNCTION:
+ errorm = format_elog_string("function %s already exists in
schema \"%s\"",
+ func_signature_string_oid(objectId),
+ get_namespace_name(namespaceId));
+ break;

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