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.linnakan...@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.



--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to