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