LUCENE-10087: Validate number of dimensions and bytes per dimension f…

…or numeric SortFields. (#283)
apache · Sep 7, 2021 · 7eb35be · 7eb35be
1 parent bc161e6
commit 7eb35be
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 4 deletions.
diff --git a/lucene/core/src/java/org/apache/lucene/search/SortField.java b/lucene/core/src/java/org/apache/lucene/search/SortField.java
@@ -19,6 +19,10 @@
 import java.io.IOException;
 import java.util.Comparator;
 import java.util.Objects;
+import org.apache.lucene.document.DoublePoint;
+import org.apache.lucene.document.FloatPoint;
+import org.apache.lucene.document.IntPoint;
+import org.apache.lucene.document.LongPoint;
 import org.apache.lucene.index.DocValues;
 import org.apache.lucene.index.IndexSorter;
 import org.apache.lucene.index.SortFieldProvider;
@@ -68,25 +72,25 @@ public enum Type {
 
     /**
      * Sort using term values as encoded Integers. Sort values are Integer and lower values are at
-     * the front.
+     * the front. Fields must either be not indexed, or indexed with {@link IntPoint}.
      */
     INT,
 
     /**
      * Sort using term values as encoded Floats. Sort values are Float and lower values are at the
-     * front.
+     * front. Fields must either be not indexed, or indexed with {@link FloatPoint}.
      */
     FLOAT,
 
     /**
      * Sort using term values as encoded Longs. Sort values are Long and lower values are at the
-     * front.
+     * front. Fields must either be not indexed, or indexed with {@link LongPoint}.
      */
     LONG,
 
     /**
      * Sort using term values as encoded Doubles. Sort values are Double and lower values are at the
-     * front.
+     * front. Fields must either be not indexed, or indexed with {@link DoublePoint}.
      */
     DOUBLE,
 

diff --git a/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java b/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java
@@ -20,6 +20,7 @@
 import java.io.IOException;
 import java.util.Arrays;
 import org.apache.lucene.index.DocValues;
+import org.apache.lucene.index.FieldInfo;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.NumericDocValues;
 import org.apache.lucene.index.PointValues;
@@ -90,6 +91,26 @@ public NumericLeafComparator(LeafReaderContext context) throws IOException {
       this.docValues = getNumericDocValues(context, field);
       this.pointValues = canSkipDocuments ? context.reader().getPointValues(field) : null;
       if (pointValues != null) {
+        FieldInfo info = context.reader().getFieldInfos().fieldInfo(field);
+        if (info == null || info.getPointDimensionCount() == 0) {
+          throw new IllegalStateException(
+              "Field "
+                  + field
+                  + " doesn't index points according to FieldInfos yet returns non-null PointValues");
+        } else if (info.getPointDimensionCount() > 1) {
+          throw new IllegalArgumentException(
+              "Field " + field + " is indexed with multiple dimensions, sorting is not supported");
+        } else if (info.getPointNumBytes() != bytesCount) {
+          throw new IllegalArgumentException(
+              "Field "
+                  + field
+                  + " is indexed with "
+                  + info.getPointNumBytes()
+                  + " bytes per dimension, but "
+                  + NumericComparator.this
+                  + " expected "
+                  + bytesCount);
+        }
         this.enableSkipping = true; // skipping is enabled when points are available
         this.maxDoc = context.reader().maxDoc();
         this.maxValueAsBytes =

diff --git a/lucene/core/src/test/org/apache/lucene/search/TestSortOptimization.java b/lucene/core/src/test/org/apache/lucene/search/TestSortOptimization.java
@@ -25,6 +25,7 @@
 import org.apache.lucene.document.FloatDocValuesField;
 import org.apache.lucene.document.FloatPoint;
 import org.apache.lucene.document.IntPoint;
+import org.apache.lucene.document.IntRange;
 import org.apache.lucene.document.LongPoint;
 import org.apache.lucene.document.NumericDocValuesField;
 import org.apache.lucene.document.StoredField;
@@ -33,6 +34,7 @@
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.index.RandomIndexWriter;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.util.LuceneTestCase;
@@ -583,4 +585,49 @@ public void testDocSort() throws IOException {
     reader.close();
     dir.close();
   }
+
+  public void testPointValidation() throws IOException {
+    final Directory dir = newDirectory();
+    final RandomIndexWriter writer = new RandomIndexWriter(random(), dir);
+    Document doc = new Document();
+
+    doc.add(new IntPoint("intField", 4));
+    doc.add(new NumericDocValuesField("intField", 4));
+
+    doc.add(new LongPoint("longField", 42));
+    doc.add(new NumericDocValuesField("longField", 42));
+
+    doc.add(new IntRange("intRange", new int[] {1}, new int[] {10}));
+    doc.add(new NumericDocValuesField("intRange", 4));
+
+    writer.addDocument(doc);
+    IndexReader reader = writer.getReader();
+    writer.close();
+
+    IndexSearcher searcher = newSearcher(reader);
+    assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            searcher.search(
+                new MatchAllDocsQuery(),
+                1,
+                new Sort(new SortField("intField", SortField.Type.LONG))));
+    assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            searcher.search(
+                new MatchAllDocsQuery(),
+                1,
+                new Sort(new SortField("longField", SortField.Type.INT))));
+    assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            searcher.search(
+                new MatchAllDocsQuery(),
+                1,
+                new Sort(new SortField("intRange", SortField.Type.INT))));
+
+    reader.close();
+    dir.close();
+  }
 }