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