Re: ALTER command reworks

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ALTER command reworks
Date: 2013-01-19 22:48:50
Message-ID: CADyhKSUMW+U=EgmfmXB94yBJMdTqty2D8pPqD_pDc6jB74T7Mw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2013/1/17 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> Kohei KaiGai escribió:
>
>> This attached patch is the rebased one towards the latest master branch.
>
> Great, thanks. I played with it a bit and it looks almost done to me.
> The only issue I can find is that it lets you rename an aggregate by
> using ALTER FUNCTION, which is supposed to be forbidden. (Funnily
> enough, renaming a non-agg function with ALTER AGGREGATE does raise an
> error). Didn't immediately spot the right place to add a check.
>
> I think these two error cases ought to have regression tests of their
> own.
>
> I attach a version with my changes.
>
The patch itself looks to me good.

About ALTER FUNCTION towards aggregate function, why we should raise
an error strictly? Some code allows to modify properties of aggregate function
specified with FUNCTION qualifier.

postgres=# COMMENT ON FUNCTION max(int) IS 'maximum number of integer';
COMMENT
postgres=# COMMENT ON AGGREGATE in4eq(int,int) IS 'comparison of integers';
ERROR: aggregate in4eq(integer, integer) does not exist

I think using AGGREGATE towards regular function is wrong, but it is not
100% wrong to use FUNCTION towards aggregate "function".
In addition, aggregate function and regular function share same namespace,
so it never makes problem even if we allows to identify aggregate function
using ALTER FUNCTION.

How about your opinion? Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Janes 2013-01-19 22:58:32 Re: lazy_vacuum_heap()'s removal of HEAPTUPLE_DEAD tuples
Previous Message Dickson S. Guedes 2013-01-19 22:34:59 Re: Review: Patch to compute Max LSN of Data Pages