Horton

Re: [PATCH] Improve geometric types

Emre Hasegeli <emre@hasegeli.com>

View thread View on postgr.es
From: Emre Hasegeli <emre@hasegeli.com>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Thomas Munro <thomas.munro@enterprisedb.com>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2018-07-31T15:14:15Z
Lists: pgsql-hackers 
> 1) common_entry_cmp is still handling 'delta' as double, although the
> CommonEntry was modified to use float8. IMHO it should also simply call
> float8_cmp_internal instead of doing comparisons.

I am changing it to define delta as "float8" and to use float8_cmp_internal().

> 2) gist_box_picksplit does this
>
>    int     m = ceil(LIMIT_RATIO * (float8) nentries);
>
> instead of
>
>    int     m = ceil(LIMIT_RATIO * (double) nentries);
>
> which seems rather unnecessary, considering the only point of the cast was
> probably to do more accurate multiplication. And it seems pointless to cast
> it to float8 but then not use float8_mul().

I am removing the cast.

> 3) computeDistance does this:
>
>     if (point->y > box->high.y)
>         result = float8_mi(point->y, box->high.y);
>     else if (point->y < box->low.y)
>         result = float8_mi(box->low.y, point->y);
>
> which seems suspicious. Shouldn't the comparisons be done by float8_lt and
> float8_gt too? That's what we do elsewhere.

I assumed the GiST code already handles NaNs correctly and tried not
to change its behavior.  It may be a good idea to revert existing NaN
handling in favour of using the inline functions every time.  Should I
do that?

> 4) I think we should just get rid of GEODEBUG entirely. The preceding
> patches removes about 20 out of 27 occurrences anyway, so let's ditch the
> remaining few.

I agree.  Shall I append it to this patch?

Commits

  1. Improve test coverage of geometric types

  2. Fix problems in handling the line data type

  3. Use the built-in float datatypes to implement geometric types

  4. Remove remaining GEODEBUG references from geo_ops.c

  5. Provide separate header file for built-in float types

  6. Refactor geometric functions and operators

  7. Fix crash in close_ps() for NaN input coordinates.

  8. Fix GiST index build for NaN values in geometric types.

  9. Enable building with Visual Studion 2013.

  10. Suppress -0 in the C field of lines computed by line_construct_pts().