Hi! On Sun, Nov 4, 2012 at 11:41 PM, Jeff Davis <pg...@j-davis.com> wrote:
> 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. > Fixed. > * Comment should indicate more explicitly that bounds_adjacent is > sensitive to the argument order. > Fixed. > * 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). > Fixed. > * I could be getting confused, but I think that line 645 of > rangetypes_spgist.c should be inverted (!bounds_adjacent(...)). > Good catch! Actually, code was in direct contradiction with comment. Fixed. > * 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. > Yes, it's a some kind of hack now. Comment is added. ------ With best regards, Alexander Korotkov.
range_spgist_adjacent-0.4.patch.gz
Description: GNU Zip compressed data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
