LUCENE-10377: Replace 'sortPos' with 'enableSkipping' in SortField.getComparator() (#603)

The sort position parameter in SortField.getComparator() is only ever used
to determine whether or not skipping should be enabled on a given comparator,
so the parameter name should reflect that.  This commit also explicitly disables
skipping in a number of cases where it is never used, in particular CheckIndex
and the grouping collectors.

Author: romseygeek

lucene/CHANGES.txt

@@ -25,6 +25,10 @@ API Changes
 
 * LUCENE-10349: WordListLoader methods now return unmodifiable CharArraySets.  (Uwe Schindler)
 
+* LUCENE-10377: SortField.getComparator() has changed signature. The second parameter is now
+  a boolean indicating whether or not skipping should be enabled on the comparator. 
+  (Alan Woodward)
+
 New Features
 ---------------------
 
@@ -143,6 +147,9 @@ Bug Fixes
 * LLUCENE-10353: Add random null injection to TestRandomChains. (Robert Muir,
   Uwe Schindler)
 
+* LUCENE-10377: CheckIndex could incorrectly throw an error when checking index sorts
+  defined on older indexes. (Alan Woodward)
+
 Other
 ---------------------
 

lucene/core/src/java/org/apache/lucene/document/FeatureSortField.java

@@ -44,7 +44,7 @@ public FeatureSortField(String field, String featureName) {
   }
 
   @Override
-  public FieldComparator<?> getComparator(int numHits, int sortPos) {
+  public FieldComparator<?> getComparator(int numHits, boolean enableSkipping) {
     return new FeatureComparator(numHits, getField(), featureName);
   }
 

lucene/core/src/java/org/apache/lucene/document/LatLonPointSortField.java

@@ -38,7 +38,7 @@ final class LatLonPointSortField extends SortField {
   }
 
   @Override
-  public FieldComparator<?> getComparator(int numHits, int sortPos) {
+  public FieldComparator<?> getComparator(int numHits, boolean enableSkipping) {
     return new LatLonPointDistanceComparator(getField(), latitude, longitude, numHits);
   }
 

lucene/core/src/java/org/apache/lucene/document/XYPointSortField.java

@@ -35,7 +35,7 @@ final class XYPointSortField extends SortField {
   }
 
   @Override
-  public FieldComparator<?> getComparator(int numHits, int sortPos) {
+  public FieldComparator<?> getComparator(int numHits, boolean enableSkipping) {
     return new XYPointDistanceComparator(getField(), x, y, numHits);
   }
 

lucene/core/src/java/org/apache/lucene/index/CheckIndex.java

@@ -1107,7 +1107,7 @@ public static Status.IndexSortStatus testSort(
 
       for (int i = 0; i < fields.length; i++) {
         reverseMul[i] = fields[i].getReverse() ? -1 : 1;
-        comparators[i] = fields[i].getComparator(1, i).getLeafComparator(readerContext);
+        comparators[i] = fields[i].getComparator(1, false).getLeafComparator(readerContext);
       }
 
       int maxDoc = reader.maxDoc();

lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java

@@ -25,7 +25,6 @@
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.NumericDocValues;
-import org.apache.lucene.index.PointValues;
 import org.apache.lucene.search.comparators.DoubleComparator;
 
 /**
@@ -483,8 +482,8 @@ void setMissingValue(double missingValue) {
 
     @Override
     public FieldComparator<Double> newComparator(
-        String fieldname, int numHits, int sortPos, boolean reversed) {
-      return new DoubleComparator(numHits, fieldname, missingValue, reversed, sortPos) {
+        String fieldname, int numHits, boolean enableSkipping, boolean reversed) {
+      return new DoubleComparator(numHits, fieldname, missingValue, reversed, false) {
         @Override
         public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException {
           DoubleValuesHolder holder = new DoubleValuesHolder();
@@ -499,11 +498,6 @@ protected NumericDocValues getNumericDocValues(
               return asNumericDocValues(holder, Double::doubleToLongBits);
             }
 
-            @Override
-            protected PointValues getPointValues(LeafReaderContext context, String field) {
-              return null;
-            }
-
             @Override
             public void setScorer(Scorable scorer) throws IOException {
               holder.values = producer.getValues(ctx, fromScorer(scorer));

lucene/core/src/java/org/apache/lucene/search/FieldComparatorSource.java

@@ -30,5 +30,5 @@ public abstract class FieldComparatorSource {
    * @return FieldComparator.
    */
   public abstract FieldComparator<?> newComparator(
-      String fieldname, int numHits, int sortPos, boolean reversed);
+      String fieldname, int numHits, boolean enableSkipping, boolean reversed);
 }

lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java

@@ -134,7 +134,7 @@ private FieldValueHitQueue(SortField[] fields, int size) {
     for (int i = 0; i < numComparators; ++i) {
       SortField field = fields[i];
       reverseMul[i] = field.reverse ? -1 : 1;
-      comparators[i] = field.getComparator(size, i);
+      comparators[i] = field.getComparator(size, i == 0);
     }
   }
 

lucene/core/src/java/org/apache/lucene/search/LongValuesSource.java

@@ -22,7 +22,6 @@
 import org.apache.lucene.index.DocValues;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.NumericDocValues;
-import org.apache.lucene.index.PointValues;
 import org.apache.lucene.search.comparators.LongComparator;
 
 /**
@@ -328,8 +327,8 @@ void setMissingValue(long missingValue) {
 
     @Override
     public FieldComparator<Long> newComparator(
-        String fieldname, int numHits, int sortPos, boolean reversed) {
-      return new LongComparator(numHits, fieldname, missingValue, reversed, sortPos) {
+        String fieldname, int numHits, boolean enableSkipping, boolean reversed) {
+      return new LongComparator(numHits, fieldname, missingValue, reversed, false) {
         @Override
         public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException {
           LongValuesHolder holder = new LongValuesHolder();
@@ -344,11 +343,6 @@ protected NumericDocValues getNumericDocValues(
               return asNumericDocValues(holder);
             }
 
-            @Override
-            protected PointValues getPointValues(LeafReaderContext context, String field) {
-              return null;
-            }
-
             @Override
             public void setScorer(Scorable scorer) throws IOException {
               holder.values = producer.getValues(ctx, DoubleValuesSource.fromScorer(scorer));

lucene/core/src/java/org/apache/lucene/search/SortField.java

@@ -495,44 +495,44 @@ public Comparator<BytesRef> getBytesComparator() {
    *
    * @lucene.experimental
    * @param numHits number of top hits the queue will store
-   * @param sortPos position of this SortField within {@link Sort}. The comparator is primary if
-   *     sortPos==0, secondary if sortPos==1, etc. Some comparators can optimize themselves when
-   *     they are the primary sort.
+   * @param enableSkipping true if the comparator can skip documents via {@link
+   *     LeafFieldComparator#competitiveIterator()}
    * @return {@link FieldComparator} to use when sorting
    */
-  public FieldComparator<?> getComparator(final int numHits, final int sortPos) {
+  public FieldComparator<?> getComparator(final int numHits, boolean enableSkipping) {
     final FieldComparator<?> fieldComparator;
     switch (type) {
       case SCORE:
         fieldComparator = new FieldComparator.RelevanceComparator(numHits);
         break;
 
       case DOC:
-        fieldComparator = new DocComparator(numHits, reverse, sortPos);
+        fieldComparator = new DocComparator(numHits, reverse, enableSkipping);
         break;
 
       case INT:
         fieldComparator =
-            new IntComparator(numHits, field, (Integer) missingValue, reverse, sortPos);
+            new IntComparator(numHits, field, (Integer) missingValue, reverse, enableSkipping);
         break;
 
       case FLOAT:
         fieldComparator =
-            new FloatComparator(numHits, field, (Float) missingValue, reverse, sortPos);
+            new FloatComparator(numHits, field, (Float) missingValue, reverse, enableSkipping);
         break;
 
       case LONG:
-        fieldComparator = new LongComparator(numHits, field, (Long) missingValue, reverse, sortPos);
+        fieldComparator =
+            new LongComparator(numHits, field, (Long) missingValue, reverse, enableSkipping);
         break;
 
       case DOUBLE:
         fieldComparator =
-            new DoubleComparator(numHits, field, (Double) missingValue, reverse, sortPos);
+            new DoubleComparator(numHits, field, (Double) missingValue, reverse, enableSkipping);
         break;
 
       case CUSTOM:
         assert comparatorSource != null;
-        fieldComparator = comparatorSource.newComparator(field, numHits, sortPos, reverse);
+        fieldComparator = comparatorSource.newComparator(field, numHits, enableSkipping, reverse);
         break;
 
       case STRING: