Re: [PATCH] Revive line type

<div dir="ltr">Hi,<br /><br />I had a look over this patch and here are my review points:<br /><br />1. Patch applies
cleanly.<br/>2. make, make install and make check is good.<br />3. I did lot of random testing and didn't find any
issue.<br/> 4. Test coverage is very well. It has all scenarios and all operators are<br />tested with line. That's
reallygreat.<br /><br />So no issues from my side.<br /><br />However, do we still need this in close_pl() ?<br /><br
/><spanstyle="font-family:courier new,monospace">#ifdef NOT_USED<br />     if (FPeq(line->A, -1.0) &&
FPzero(line->B))<br/>    {                            /* vertical */<br />    }<br />#endif</span><br /><br />Also
close_sl,close_lb and dist_lb are NOT yet implemented. It will be good<br /> if we have those. But I don't think we
shouldwait for those functions to be<br />implemented. We can go ahead with this. Please confirm above concern so
that<br/>I will mark it as "Ready for Committer".<br /><br /> Thanks<br /><div class="gmail_extra"><br /><br /><div
class="gmail_quote">OnTue, Sep 17, 2013 at 7:34 AM, Alvaro Herrera <span dir="ltr"><<a
href="mailto:alvherre@2ndquadrant.com"target="_blank">alvherre@2ndquadrant.com</a>></span> wrote:<br /><blockquote
class="gmail_quote"style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">David Fetter escribió:<br
/><divclass="im"><br /> > Should the things you tried and others be in the regression tests?  If<br /> > so,
shouldwe start with whatever had been in the regression tests<br /> > when the line type was dropped?<br /><br
/></div>Actually,the patch does include a regression test for the revived type<br /> (and it passes).  I don't think
morethan that is needed.<br /><div class="im HOEnZb"><br /> --<br /> Álvaro Herrera                <a
href="http://www.2ndQuadrant.com/"target="_blank">http://www.2ndQuadrant.com/</a><br /> PostgreSQL Development, 24x7
Support,Training & Services<br /><br /><br /></div><div class="HOEnZb"><div class="h5">--<br /> Sent via
pgsql-hackersmailing list (<a href="mailto:pgsql-hackers@postgresql.org">pgsql-hackers@postgresql.org</a>)<br /> To
makechanges to your subscription:<br /><a href="http://www.postgresql.org/mailpref/pgsql-hackers"
target="_blank">http://www.postgresql.org/mailpref/pgsql-hackers</a><br/></div></div></blockquote></div><br /><br
clear="all"/><br />-- <br /><div dir="ltr">Jeevan B Chalke<br />Principal Software Engineer, Product Development<br
/>EnterpriseDBCorporation<br /><br /></div></div></div>