[PATCH] Improve geometric types

From: Emre Hasegeli <emre(at)hasegeli(dot)com>
To: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: [PATCH] Improve geometric types
Date: 2017-06-01 14:56:27
Message-ID: CAE2gYzxF7-5djV6-cEvqQu-fNsnt=EqbOURx7ZDg+Vv6ZMTWbg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

This is my next attempt to bring more sanity to the geometric types.
After the previous one [1] went nowhere, I extracted the parts I can
fix without touching the EPSILON. I still hope to improve the
problems around EPSILON after this. I organised the changes as 4
patches for ease of review:

== geo-funcs-v1 ==

Refactor geometric functions and operators code

The geometric types were not using each other's functions. I believe
the reason behind this is simpler types line point and line being
developed after more complicated ones. This patch reduces duplicate
code and makes functions of different datatypes more compatible. We can
do much better than that, but it would require touching many more lines.
The changes can be summarised as:

* Re-use more functions to implement others
* Unify *_construct functions to obtain pre-allocated memory
* Unify *_interpt_internal functions to obtain pre-allocated memory
* Remove private functions from geo_decls.h
* Switch using C11 hypot() as the comment suggested

src/backend/utils/adt/geo_ops.c | 810 +++++++++++++-------------------
src/include/utils/geo_decls.h | 12 +-
src/test/regress/regress.c | 11 +-
3 files changed, 345 insertions(+), 488 deletions(-)

== float-header-v04 ==

Provide header file for built-in float datatypes

Even though, some datatypes under adt/ have separate header files,
most of the simple ones do not. Their public functions were on
the builtins.h. We would need to make more functions of floats public
to let the geometric types built on top of them. This is a good
opportunity to make a separate header file for floats.

1acf7572554515b99ef6e783750aaea8777524ec made _cmp functions public
to solve NaN problem locally for GiST indexes. This patch reworks it
in favour of a more extensive API. Kevin Grittner suggested to design
the API using inline functions. They are easier to use compared
to macros, and avoid double-evaluation hazards.

contrib/btree_gin/btree_gin.c | 3 +-
contrib/btree_gist/btree_ts.c | 2 +-
contrib/cube/cube.c | 2 +-
contrib/postgres_fdw/postgres_fdw.c | 2 +-
src/backend/access/gist/gistget.c | 2 +-
src/backend/access/gist/gistproc.c | 55 +-
src/backend/access/gist/gistutil.c | 2 +-
src/backend/utils/adt/float.c | 593 ++++--------------
src/backend/utils/adt/formatting.c | 8 +-
src/backend/utils/adt/geo_ops.c | 6 +-
src/backend/utils/adt/geo_spgist.c | 2 +-
src/backend/utils/adt/numeric.c | 1 +
src/backend/utils/adt/rangetypes_gist.c | 3 +-
src/backend/utils/adt/rangetypes_selfuncs.c | 3 +-
src/backend/utils/adt/rangetypes_typanalyze.c | 3 +-
src/backend/utils/adt/timestamp.c | 1 +
src/backend/utils/misc/guc.c | 1 +
src/include/utils/builtins.h | 14 -
src/include/utils/float.h | 383 +++++++++++
src/include/utils/geo_decls.h | 1 +
20 files changed, 561 insertions(+), 526 deletions(-)

== geo-float-v1 ==

Use the built-in float datatype to implement geometric types

This will provide:

* Check for underflow and overflow
* Check for division by zero
* Handle NaNs consistently

The patch also replaces all occurrences of "double" as "float8". They
are the same, but were randomly spread around on the same file.

src/backend/access/gist/gistproc.c | 156 +++++----
src/backend/utils/adt/geo_ops.c | 546 +++++++++++++++--------------
src/backend/utils/adt/geo_spgist.c | 36 +-
src/include/utils/float.h | 13 +
src/include/utils/geo_decls.h | 40 +--
5 files changed, 412 insertions(+), 379 deletions(-)

== line-fixes-v1 ==

Fix obvious problems around the line datatype

I have noticed some line operators retuning wrong results, and Tom Lane
spotted similar problems on more places. Source history reveals that
during 1990s, the internal format of the line datatype is changed, but
most functions haven't got the hint. The fixes are:

* Make operators more symmetric
* Reject invalid specification A=B=0 on receive
* Avoid division by zero on perpendicular operator
* Fix intersection and distance operators when neither A nor B is 1
* Avoid point distance operator crashing on float precision loss
* Fix line segment distance by getting the minimum as the comment suggested

Previous discussion:
https://www.postgresql.org/message-id/flat/CAE2gYzw_-z%3DV2kh8QqFjenu%3D8MJXzOP44wRW%3DAzzeamrmTT1%3DQ%40mail.gmail.com

src/backend/utils/adt/geo_ops.c | 115 +++++++++++++++++++++-----------
1 file changed, 77 insertions(+), 38 deletions(-)

[1] https://www.postgresql.org/message-id/flat/CAE2gYzwwxPWbzxY3mtN4WL7W0DCkWo8gnB2ThUHU2XQ9XwgHMg%40mail.gmail.com

Attachment Content-Type Size
0001-geo-funcs-v1.patch application/octet-stream 67.5 KB
0002-float-header-v04.patch application/octet-stream 84.2 KB
0003-geo-float-v1.patch application/octet-stream 101.0 KB
0004-line-fixes-v1.patch application/octet-stream 9.0 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-06-01 15:14:57 Re: [JDBC] Channel binding support for SCRAM-SHA-256
Previous Message Robert Haas 2017-06-01 14:54:30 Re: <> join selectivity estimate question