Re: [HACKERS] [PATCH] Improve geometric types

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: emre(at)hasegeli(dot)com
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, alvherre(at)alvh(dot)no-ip(dot)org, robertmhaas(at)gmail(dot)com, a(dot)alekseev(at)postgrespro(dot)ru, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [HACKERS] [PATCH] Improve geometric types
Date: 2018-01-31 04:09:09
Message-ID: 20180131.130909.210233873.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Sun, 21 Jan 2018 21:59:19 +0100, Emre Hasegeli <emre(at)hasegeli(dot)com> wrote in <CAE2gYzxDYs5tcvc4uErsWaFTb3UTYS0ERt_fFyi-28Ldvs5d4A(at)mail(dot)gmail(dot)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..

regards

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Lakhin 2018-01-31 05:47:57 A typo in error message
Previous Message Haozhou Wang 2018-01-31 03:57:12 [PATCH] Add missing type conversion functions for PL/Python