Re: SP-GiST for ranges based on 2d-mapping and quad-tree

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