On 13.07.2012 02:00, Alexander Korotkov wrote:
> Done. There are separate patch for get rid of TrickFunctionCall2 and
> version of SP-GiST for ranges based on that patch.
Looking at the SP-GiST patch now..
It would be nice to have an introduction, perhaps as a file comment at
the top of rangetypes_spgist.c, explaining how the quad tree works. I
have a general idea of what a quad tree is, but it's not immediately
obvious how it maps to SP-GiST. What is stored on a leaf node and an
internal node? What is the 'prefix' (seems to be the centroid)? How are
ranges mapped to 2D points? (the function comment of getQuadrant() is a
good start for that last one)
In spg_range_quad_inner_consistent(), if in->hasPrefix == true, ISTM
that in all cases where 'empty' is true, 'which' is set to 0, meaning
that there can be no matches in any of the quadrants. In most of the
case-branches, you explicitly check for 'empty', but even in the ones
where you don't, I think you end up setting which=0 if empty==true. I'm
not 100% sure about the RANGESTRAT_ADJACENT case, though. Am I missing
something?
It would be nice to avoid the code duplication between the new
bounds_adjacent() function, and the range_adjacent_internal(). Perhaps
move bounds_adjacent() to rangetypes.c and use it in
range_adjacent_internal() too?
-- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com