Hi Emre,
Now that the buildfarm is no longer complaining about 0001 and 0002, I'm
working on reviewing and committing 0003. It seems quite straightforward
but I do have couple of comment/questions:
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.
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().
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.
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.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
