Re: [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: Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Improve geometric types
Date: 2017-10-02 09:46:15
Message-ID: CAE2gYzz7wiUnyb1TxCk5GzXt6j7efzGmMgH0gTe8xwKZ=FAF5A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> 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

Attachment Content-Type Size
0001-geo-funcs-v04.patch application/octet-stream 75.7 KB
0002-float-header-v08.patch application/octet-stream 84.2 KB
0003-geo-float-v05.patch application/octet-stream 99.3 KB
0004-line-fixes-v04.patch application/octet-stream 9.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2017-10-02 09:53:16 Re: [PATCH] Assert that the correct locks are held when calling PageGetLSN()
Previous Message Daniel Gustafsson 2017-10-02 09:36:27 Re: Patch: Write Amplification Reduction Method (WARM)