Hi,
I had a look over this patch and here are my review points:
1. Patch applies cleanly.
2. make, make install and make check is good.
3. I did lot of random testing and didn't find any issue.
4. Test coverage is very well. It has all scenarios and all operators are
tested with line. That's really great.
So no issues from my side.
However, do we still need this in close_pl() ?
#ifdef NOT_USED
if (FPeq(line->A, -1.0) && FPzero(line->B))
{ /* vertical */
}
#endif
Also close_sl, close_lb and dist_lb are NOT yet implemented. It will be good
if we have those. But I don't think we should wait for those functions to be
implemented. We can go ahead with this. Please confirm above concern so that
I will mark it as "Ready for Committer".
Thanks
On Tue, Sep 17, 2013 at 7:34 AM, Alvaro Herrera <alvhe...@2ndquadrant.com>wrote:
> David Fetter escribió:
>
> > Should the things you tried and others be in the regression tests? If
> > so, should we start with whatever had been in the regression tests
> > when the line type was dropped?
>
> Actually, the patch does include a regression test for the revived type
> (and it passes). I don't think more than that is needed.
>
> --
> Álvaro Herrera http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
