From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | emre(at)hasegeli(dot)com |
Cc: | a(dot)alekseev(at)postgrespro(dot)ru, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] Improve geometric types |
Date: | 2017-10-03 07:18:11 |
Message-ID: | 20171003.161811.65352202.horiguchi.kyotaro@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks.
At Mon, 2 Oct 2017 11:46:15 +0200, Emre Hasegeli <emre(at)hasegeli(dot)com> wrote in <CAE2gYzz7wiUnyb1TxCk5GzXt6j7efzGmMgH0gTe8xwKZ=FAF5A(at)mail(dot)gmail(dot)com>
> > 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.
Yeah, it is not about the patch itself.
> > # 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().
I see. It's fine with me. I suppose that Tom didn't intened the
suggestion to be teken literary so using FPzero() seems better
(at least for now).
> > 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.
Mmm, sorry. It's my mistake.
> [1] https://www.postgresql.org/message-id/11053.1466362319%40sss.pgh.pa.us
I'll look the new version further.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Vladimir Sitnikov | 2017-10-03 07:30:00 | Re: 64-bit queryId? |
Previous Message | Andreas Seltenreich | 2017-10-03 07:04:50 | Re: [sqlsmith] crash in RestoreLibraryState during low-memory testing |