Re: [HACKERS] [PATCH] Improve geometric types

From: Emre Hasegeli <emre(at)hasegeli(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] [PATCH] Improve geometric types
Date: 2018-02-01 09:35:42
Message-ID: CAE2gYzzqynBViwKbauxKC1VGXwDnURTm7KR+=CXZpSiRLWscEA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> 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);

Fixing.

> 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.

It is indeed redundant. I am removing it.

> 3. point_sl can return -0.0 and that is a thing that this patch
> intends to avoid. line_invsl has the same problem.

The existing version of the function has the same issue. I think we
need a global solution for -0s. See the float-zero patch.

> 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.

Yes, I am changing it.

> > 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.

Me neither, but it is probably better than returning a point that
extends the endpoints of the segment. I am inclined to leave it alone
for now.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Emre Hasegeli 2018-02-01 09:36:41 Re: [HACKERS] [PATCH] Improve geometric types
Previous Message Simon Riggs 2018-02-01 09:32:02 Re: [HACKERS] Surjective functional indexes