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

Ops., it was a bug: RANGESTRAT_ADJACENT shoud set which=0 if empty==true,
while RANGESTRAT_CONTAINS and RANGESTRAT_CONTAINED_BY not. Corrected.

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?


Done.

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