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

<div dir="ltr"><br /><div class="gmail_extra"><div class="gmail_quote">On Tue, Jun 3, 2014 at 10:48 AM, Andres Freund
<spandir="ltr"><<a href="mailto:andres@2ndquadrant.com" target="_blank">andres@2ndquadrant.com</a>></span>
wrote:<br/><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"><divclass="">On 2014-06-03 10:37:53 -0400, Tom Lane wrote:<br /> > Andres Freund
<<ahref="mailto:andres@2ndquadrant.com">andres@2ndquadrant.com</a>> writes:<br /> > > On 2014-06-03
10:24:46-0400, Tom Lane wrote:<br /> > >> Personally, I would wonder why the regression tests contain such a
query<br/> > >> in the first place.  It seems like nothing but a major maintenance PITA.<br /> ><br /> >
>I haven't added it, but it seems appropriate in that specific case. The<br /> > > number of leakproof
functionsshould be fairly small and every addition<br /> > > should be carefully reviewed... I am e.g. not sure
thatit's a good idea<br /> > > to declare network_smaller/greater as leakproof - but it's hard to catch<br />
>> that on the basic of pg_proc.h alone.<br /> ><br /> > Meh.  I agree that new leakproof functions should
becarefully reviewed,<br /> > but I have precisely zero faith that this regression test will contribute<br /> >
tothat.<br /><br /></div>Well, I personally wouldn't have noticed that the OP's patch marked the<br /> new functions as
leakproofwithout that test. At least not while looking<br /> at the patch. pg_proc.h is just too hard to read.<br
/><divclass=""><br /> > It hasn't even got a comment saying why changes here should<br /> > receive any scrutiny;
moreover,it's not in a file where changes would be<br /> > likely to excite suspicion.  (Probably it should be in
opr_sanity,if<br /> > we're going to have such a thing at all.)<br /><br /></div>I don't object to moving it
there.<br/><div class=""><div class="h5"><br /> Greetings,<br /><br /> Andres Freund<br /><br /> --<br />  Andres
Freund                    <a href="http://www.2ndQuadrant.com/" target="_blank">http://www.2ndQuadrant.com/</a><br />
 PostgreSQLDevelopment, 24x7 Support, Training & Services<br /></div></div></blockquote></div><br /><br
/></div><divclass="gmail_extra">Andres's changes on June 3rd to <br /><a
href="https://github.com/postgres/postgres/commits/master/src/test/regress/expected/create_function_3.out">https://github.com/postgres/postgres/commits/master/src/test/regress/expected/create_function_3.out</a><br
/></div><divclass="gmail_extra">are causing patch v2 to fail for that regression test file. <br /><br />postgres $
patch-p1 -i ../inet_agg_v2.patch <br />patching file src/backend/utils/adt/network.c<br />patching file
src/include/catalog/pg_aggregate.h<br/> patching file src/include/catalog/pg_proc.h<br />patching file
src/include/utils/builtins.h<br/>patching file src/test/regress/expected/create_function_3.out<br />Hunk #1 FAILED at
153.<br/>1 out of 1 hunk FAILED -- saving rejects to file src/test/regress/expected/create_function_3.out.rej<br />
patchingfile src/test/regress/expected/inet.out<br />patching file src/test/regress/sql/inet.sql<br /><br />Otherwise
itapplies without any issues to the latest HEAD. I built and started a new instance, and I was able to test at least
thebasic min/max functionality<br /><br />keith=# create table test_inet (id serial, ipaddress inet);<br />CREATE
TABLE<br/>Time: 25.546 ms<br />keith=# insert into test_inet (ipaddress) values ('192.168.1.1');<br />INSERT 0 1<br
/>Time:3.143 ms<br />keith=# insert into test_inet (ipaddress) values ('192.168.1.2');<br /> INSERT 0 1<br />Time:
2.932ms<br />keith=# insert into test_inet (ipaddress) values ('127.0.0.1');<br />INSERT 0 1<br />Time: 1.786 ms<br
/>keith=#select min(ipaddress) from test_inet;<br />    min    <br />-----------<br /> 127.0.0.1<br /> (1 row)<br /><br
/>Time:3.371 ms<br />keith=# select max(ipaddress) from test_inet;<br />     max     <br />-------------<br
/> 192.168.1.2<br/>(1 row)<br /><br />Time: 1.104 ms<br /><br /></div><div class="gmail_extra">--<br />Keith Fiske<br
/>Database Administrator<br />OmniTI Computer Consulting, Inc.<br /><a href="http://www.keithf4.com"
target="_blank">http://www.keithf4.com</a><br/></div></div>