> > Hmm, the second for loop in gseg_picksplit uses "i < maxoff" whereas the > other one uses <=. The first is probably correct; if the second is also > correct it merits a comment on the discrepancy (To be honest, I'd get > rid of the "-1" in computing maxoff and use < in both places, given that > offsets are 1-indexed). Also, the second one is using i++ to increment; > probably should be OffsetNumberNext just to stay consistent with the > rest of the code. > 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.
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. ---- With best regards, Alexander Korotkov.
