Re: BUG #9652: inet types don't support min/max

On Thu, May 29, 2014 at 3:28 AM, Keith Fiske <keith@omniti.com> wrote:
> On Sun, Mar 23, 2014 at 10:42 PM, Haribabu Kommi <kommi.haribabu@gmail.com>
> wrote:
>>
>> On Fri, Mar 21, 2014 at 5:17 PM,  <darius@dons.net.au> wrote:
>> > The following bug has been logged on the website:
>> > reclog=> select * from foo;
>> >    bar
>> > ---------
>> >  1.2.3.4
>> > (1 row)
>> >
>> > reclog=> select min(bar) from foo;
>> > ERROR:  function min(inet) does not exist
>> > LINE 1: select min(bar) from foo;
>> >                ^
>> > HINT:  No function matches the given name and argument types. You might
>> > need
>> > to add explicit type casts.
>> >
>> > This is surprising to me since the inet type is ordered, hence min/max
>> > are
>> > possible.
>>
>> In the code, some comparison logic for the inet datatypes already present.
>> With those I thought it is easy to support the min and max aggregates
>> also.
>> So I modified the code to support the aggregate functions of min and
>> max by using
>> the already existing inet comparison logic. Is there anything I am
>> missing?
>>
>> postgres=# create table tbl(f inet);
>> CREATE TABLE
>> postgres=# insert into tbl values('1.2.3.5');
>> INSERT 0 1
>> postgres=# insert into tbl values('1.2.3.4');
>> INSERT 0 1
>> postgres=# select min(f) from tbl;
>>    min
>> ---------
>>  1.2.3.4
>> (1 row)
>>
>> postgres=# select max(f) from tbl;
>>    max
>> ---------
>>  1.2.3.5
>> (1 row)
>>
>> Patch is attached.
>
> This is my first time reviewing a patch, so apologies if I've missed
> something in the process.
>
> I tried applying your patch to the current git HEAD as of 2014-05-27 and the
> portion against pg_aggregate.h fails
>
> postgres $ patch -Np1 -i ../inet_agg.patch
> patching file src/backend/utils/adt/network.c
> Hunk #1 succeeded at 471 (offset -49 lines).
> patching file src/include/catalog/pg_aggregate.h
> Hunk #1 FAILED at 140.
> Hunk #2 FAILED at 162.
> 2 out of 2 hunks FAILED -- saving rejects to file
> src/include/catalog/pg_aggregate.h.rej
> patching file src/include/catalog/pg_proc.h
> Hunk #1 succeeded at 2120 (offset 8 lines).
> Hunk #2 succeeded at 3163 (offset 47 lines).
> Hunk #3 succeeded at 3204 (offset 47 lines).
> patching file src/include/utils/builtins.h
> Hunk #1 succeeded at 907 (offset 10 lines).
>
> Looks like starting around April 12th some additional changes were made to
> the DATA lines in this file that have to be accounted for.
>
> https://github.com/postgres/postgres/commits/master/src/include/catalog/pg_aggregate.h
>
> Don't have the knowledge of how to fix this for the new format, but thought
> I'd at least try to apply the patch and see if it works.

Thanks for verifying the patch. Please find the re-based patch
attached in the mail.

Regards,
Hari Babu
Fujitsu Australia