From: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
---|---|
To: | Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
Cc: | Jeff Davis <pgsql(at)j-davis(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: SP-GiST for ranges based on 2d-mapping and quad-tree |
Date: | 2012-08-06 14:49:35 |
Message-ID: | 501FD97F.505@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Just to check where we stand on this: Are you going to send a finalized
version of this patch, based on the one I sent earlier, or should I pick
up that version and try to get it into committable state?
On 23.07.2012 10:37, Alexander Korotkov wrote:
> On Fri, Jul 20, 2012 at 3:48 PM, Heikki Linnakangas<
> heikki(dot)linnakangas(at)enterprisedb(dot)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.
>
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2012-08-06 15:09:09 | Re: WIP patch for LATERAL subqueries |
Previous Message | Tom Lane | 2012-08-06 14:47:53 | Re: WIP patch for LATERAL subqueries |