Re: Fix for seg picksplit function

On 2010-11-10 14:53, Alexander Korotkov wrote:
>
>
> Actually I can't understand the purpose of FirstOffsetNumber
> and OffsetNumberNext macros. When I wrote the patch I though about
> sortItems as about "clean from all these strange things" array, that's
> why I didn't use OffsetNumberNext there. :)
> I see only way to save logic of these macros is to use array starting
> from FirstOffsetNumber index like in gbt_num_picksplit.
Another reason for not using is FirstOffsetNumber and it's related
macro's on the qsort array, is that InvalidOffsetNumber (0) is not
invalid for the array.
However all other sorts in picksplit functions already seem to do it
this way. I'm not sure it's wise to introduce a different approach.
>
> The assignment to *left and *right at the end of the routine seem
> pretty
> useless (not to mention the comment talking about a routine that
> doesn't
> exist anywhere).
>
> I found, that gtrgm_picksplit in pg_trgm and gtsvector_picksplit in
> core still use this assignment, while gist_box_picksplit
> and gbt_num_picksplit not. If this assignment is overall useless, than
> I think we should remove it from gtrgm_picksplit
> and gtsvector_picksplit in order to not mislead developers of gist
> implementations.
+1

regards,
Yeb Havinga