Skip site navigation (1) Skip section navigation (2)

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: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: SP-GiST for ranges based on 2d-mapping and quad-tree
Date: 2012-11-04 19:41:48
Message-ID: 1352058108.6292.13.camel@jdavis-laptop (view raw or flat)
Thread:
Lists: pgsql-hackers
On Fri, 2012-11-02 at 12:47 +0400, Alexander Korotkov wrote:

> Right version of patch is attached.
> 
* In bounds_adjacent, there's no reason to flip the labels back.
* Comment should indicate more explicitly that bounds_adjacent is
sensitive to the argument order.
* In bounds_adjacent, it appears that "bound1" corresponds to "B" in the
comment above, and "bound2" corresponds to "A" in the comment above. I
would have guessed from reading the comment that bound1 corresponded to
A. We should just use consistent names between the comment and the code
(e.g. boundA and boundB).
* I could be getting confused, but I think that line 645 of
rangetypes_spgist.c should be inverted (!bounds_adjacent(...)).
* I think needPrevious should have an explanatory comment somewhere. It
looks like you are using it to store some state as you descend the tree,
but it doesn't look like it's used to reconstruct the value (because we
already have the value anyway). Since it's being used for a purpose
other than what's intended, that should be explained.

Regards,
	Jeff Davis




In response to

Responses

pgsql-hackers by date

Next:From: Jeff DavisDate: 2012-11-04 19:59:20
Subject: Arguments to foreign tables?
Previous:From: Tom LaneDate: 2012-11-04 19:30:38
Subject: Re: Unresolved error 0xC0000409 on Windows Server

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group