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

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Subject: Re: SP-GiST for ranges based on 2d-mapping and quad-tree
Date: 2012-12-17 10:42:20
Message-ID: 1355740940.8570.11.camel@jdavis-laptop
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 2012-12-14 at 01:31 +0400, Alexander Korotkov wrote:
> Hi!
>
Hi!

I have attached a patch with some significant edits.

* In your patch, there was still an inconsistency between the comment
for bounds_adjacent and the code. I refactored it to ensure it always
takes the upper bound as boundA, and the lower bound as boundB, so that
it can invert the inclusivities to create A..B to match the comments.

* In the consistent method, you were inverting upper to be a lower bound
and lower to be an upper bound. I don't understand why (perhaps I did
the first time I read the patch), so I removed it.

* It looks like the comments for which1/2 were inconsistent with the
code. I tried to fix that.

* I significantly refactored the comments and code for the consistent
method. I had trouble understanding the original comments, particularly
around the edge cases.

Please take a look and see if it still matches your algorithm properly.
This patch is not intended to be a final version (I didn't analyze my
code carefully), but just to show you what I mean and how I interpret
what the code is trying to do. You don't need to use my refactorings,
but if not, the comments in your version need more improvement so I can
understand.

I found it easier to reason in terms of horizontal and vertical lines,
and which quadrants they crossed, and then work out the edge cases. I'm
not sure if that reasoning was correct, but it seemed to make more sense
to me.

Regards,
Jeff Davis

Attachment Content-Type Size
range_spgist_adjacent-0.4A.patch.gz application/x-gzip 3.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bernhard Schrader 2012-12-17 11:14:29 Problems with enums after pg_upgrade
Previous Message Shigeru Hanada 2012-12-17 09:44:55 Re: proposal - assign result of query to psql variable