Re: [PATCH] Improve geometric types
Emre Hasegeli <emre@hasegeli.com>
From: Emre Hasegeli <emre@hasegeli.com>
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: Aleksander Alekseev <a.alekseev@postgrespro.ru>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-10-02T09:46:15Z
Lists: pgsql-hackers
Attachments
- 0001-geo-funcs-v04.patch (application/octet-stream) patch v4-0001
- 0002-float-header-v08.patch (application/octet-stream) patch v8-0002
- 0003-geo-float-v05.patch (application/octet-stream) patch v5-0003
- 0004-line-fixes-v04.patch (application/octet-stream) patch v4-0004
> And this should be the last comment of mine on the patch set. Thank you. The new versions of the patches are attached addressing your comments. > By the way, I found that MAXDOUBLEWIDTH has two inconsistent > definitions in formatting.c(500) and float.c(128). The definition > in new float.h is according to float.c and it seems better to be > untouched and it would be another issue. The last version of the patch don't move these declarations to the header file. > # The commit message of 0001 says that "using C11 hypot()" but it > # is sine C99. I suppose we shold follow C99 at the time so I > # suggest rewrite it in the next version if any. Changed. > close_pl got a bug fix not only refactoring. I think it is > recommended to separate bugs and improvements, but I'm fine with > the current patch. I split the refactoring to the first patch. > You added sanity check "A==0 && B==0" (in Ax + By + C) in > line_recv. I'm not sure the necessity of the check since it has > been checked in line_in but anyway the comparisons seem to be > typo(or thinko) of FPzero. Tom Lane suggested [1] this one. I now made it use FPzero(). > dist_pl is changed to take the smaller distance of both ends of > the segment. It seems absorbing error, so it might be better > taking the mean of the two distances. If you have a firm reason > for the change, it is better to be written there, or it might be > better left alone. I don't really, so I left that part out. [1] https://www.postgresql.org/message-id/11053.1466362319%40sss.pgh.pa.us
Commits
-
Improve test coverage of geometric types
- a3d2844852dc 12.0 landed
-
Fix problems in handling the line data type
- 2e2a392de391 12.0 landed
-
Use the built-in float datatypes to implement geometric types
- c4c340088546 12.0 landed
-
Remove remaining GEODEBUG references from geo_ops.c
- a082aed0723c 12.0 landed
-
Provide separate header file for built-in float types
- 6bf0bc842bd7 12.0 landed
-
Refactor geometric functions and operators
- a7dc63d904a6 12.0 landed
-
Fix crash in close_ps() for NaN input coordinates.
- 278148907a97 9.6.0 cited
-
Fix GiST index build for NaN values in geometric types.
- 1acf75725545 9.6.0 cited
-
Enable building with Visual Studion 2013.
- cec8394b5ccd 9.4.0 cited
-
Suppress -0 in the C field of lines computed by line_construct_pts().
- 43fe90f66a0b 9.4.0 cited