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

On Fri, Jul 20, 2012 at 3:48 PM, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:

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)

I've added some comments at the top of rangetypes_spgist.c.

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?

------
With best regards,
Alexander Korotkov.