Re: ALTER command reworks - Mailing list pgsql-hackers

From Kohei KaiGai
Subject Re: ALTER command reworks
Date
Msg-id CADyhKSWHGJaMFTnC-vfipXcnvqmY7ni25=owNrM3bo16JMoHFg@mail.gmail.com
Whole thread Raw
In response to Re: ALTER command reworks  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: ALTER command reworks  (Alvaro Herrera <alvherre@2ndquadrant.com>)
List pgsql-hackers
2012/10/5 Alvaro Herrera <alvherre@2ndquadrant.com>:
> Kohei KaiGai escribió:
>> 2012/8/31 Kohei KaiGai <kaigai@kaigai.gr.jp>:
>> > 2012/8/30 Robert Haas <robertmhaas@gmail.com>:
>> >> On Mon, Aug 13, 2012 at 3:35 PM, Kohei KaiGai <kaigai@kaigai.gr.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@kaigai.gr.jp>

Attachment

pgsql-hackers by date:

Previous
From: Atri Sharma
Date:
Subject: Re: Regarding identifying a foreign scan
Next
From: Selena Deckelmann
Date:
Subject: Re: setting per-database/role parameters checks them against wrong context