Re: [HACKERS] [PATCH] Improve geometric types - Mailing list pgsql-hackers
From | Kyotaro HORIGUCHI |
---|---|
Subject | Re: [HACKERS] [PATCH] Improve geometric types |
Date | |
Msg-id | 20180131.173342.26333067.horiguchi.kyotaro@lab.ntt.co.jp Whole thread Raw |
In response to | Re: [HACKERS] [PATCH] Improve geometric types (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>) |
Responses |
Re: [HACKERS] [PATCH] Improve geometric types
Re: [HACKERS] [PATCH] Improve geometric types |
List | pgsql-hackers |
At Wed, 31 Jan 2018 13:09:09 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20180131.130909.210233873.horiguchi.kyotaro@lab.ntt.co.jp> > At Sun, 21 Jan 2018 21:59:19 +0100, Emre Hasegeli <emre@hasegeli.com> wrote in <CAE2gYzxDYs5tcvc4uErsWaFTb3UTYS0ERt_fFyi-28Ldvs5d4A@mail.gmail.com> > > New versions are attached including all changes we discussed. > > Thanks for the new version. > > # there's many changes from the previous version.. > > About 0001 and 0002. > > 1."COPT=-DGEODEBUG make" complains as follows. > > | geo_ops.c:2445:62: error: invalid type argument of unary ‘*’ (have ‘float8 {aka double}’) > | printf("dist_ppoly_internal- segment 0/n distance is %f\n", *result); > > 2. line_construct_pm has been renamed to line_construct. I > noticed that the patch adds the second block for "(m == 0.0)" > (from the ealier versions) but it seems to work exactly as the > same to the "else" block. We need a comment about the reason > for the seemingly redundant second block. > > 3. point_sl can return -0.0 and that is a thing that this patch > intends to avoid. line_invsl has the same problem. > > 4. lseg_interpt_line is doing as follows. > > > if (FPeq(lseg->p[0].x, interpt.x) && FPeq(lseg->p[0].y, interpt.y)) > > *result = lseg->p[0]; > > else if (FPeq(lseg->p[1].x, interpt.x) && FPeq(lseg->p[1].y, interpt.y)) > > *result = lseg->p[1]; > > I suppose we can use point_pt_point for this purpose. > > > if (point_eq_point(&lseg->p[0], &interpt)) > > *result = lseg->p[0]; > > else if (point_eq_point(&lseg->p[1], &interpt)) > > *result = lseg->p[1]; > > However I'm not sure that adjusting the intersection to the > tips of the segment is good or not. Adjusting onto the line > can be better in another case. lseg_interpt_lseg, for > instance, checks lseg_contain_point on the line parameter of > lseg_interpt_line. > > # I'll be back later.. I've been back. 0003: This patch replaces "double" with float and bare arithmetic and comparisons on double to special functions accompanied with checking against abnormal values. - Almost all of them are eliminated with a few safe exceptions. - circle_recv(), circle_distance(), dist_pc(), dist_cpoint() are using "< 0.0" comparison but it looks fine. - pg_hypot is right to use bare arithmetics. ! circle_contain_pt() does the following comparison and it seems to be out of our current policy. point_dt(center, point) <= radius I suppose this should use FPle. FPle(point_dt(center, point), radius) The same is true of circle_contain_pt(), pt_contained_circle . # Sorry, that' all for today.. regards, -- Kyotaro Horiguchi NTT Open Source Software Center
pgsql-hackers by date: