Details
-
Improvement
-
Status: Closed
-
Major
-
Resolution: Fixed
-
None
-
None
-
None
-
New
Description
SortField.getComparator() takes two parameters: the number of hits we are collecting, so that the comparator can size its internal priority queue; and the position of this sortfield in the overall sort. This second parameter is only actually used to determine whether or not the SortField should enable various skipping operations, building comparative iterators etc. However, it's not clear from the API that this is why it is passed, and as a consequence all sorts of consumers of this API faithfully pass through their sort positions even when they aren't actually using skipping at all. In particular, CheckIndex does this when checking index sorts, and this in fact leads to a bug, where it will attempt to build skipping structures over fields from 8x-created indexes that did not have the same type constraints that 9x indexes do and incorrectly throw an error.
I think we should replace this second parameter with a simple boolean, `enableSkipping`, that the TopHits collectors can pass as `true` for the first sortfield. Other APIs that use sorts but not skipping, like CheckIndex or the grouping collectors, can always pass `false` so we don't waste time building skipping structures that never get used.
Attachments
Issue Links
- links to
