> 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?