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.