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
