From 72a43d97d838986bf86ade82153ec7e2eb394aa5 Mon Sep 17 00:00:00 2001 From: jimczi Date: Fri, 10 Sep 2021 17:48:33 +0200 Subject: [PATCH 1/2] LUCENE-10089: Disable numeric sort optimization early This commit moves the responsibility to disable the numeric sort optimization on comparators to the SortField. This way we don't need to apply the logic on every top field collectors. --- .../lucene/search/FieldValueHitQueue.java | 1 - .../org/apache/lucene/search/SortField.java | 30 +++++++++++++------ .../lucene/search/SortedNumericSortField.java | 17 ++++++++--- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java b/lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java index 7fa825df0dcb..b4454034df59 100644 --- a/lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java +++ b/lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java @@ -135,7 +135,6 @@ private FieldValueHitQueue(SortField[] fields, int size) { SortField field = fields[i]; reverseMul[i] = field.reverse ? -1 : 1; comparators[i] = field.getComparator(size, i); - if (field.getOptimizeSortWithPoints() == false) comparators[i].disableSkipping(); } if (numComparators == 1) { // inform a comparator that sort is based on this single field diff --git a/lucene/core/src/java/org/apache/lucene/search/SortField.java b/lucene/core/src/java/org/apache/lucene/search/SortField.java index adfd5ef8da89..2daf59bb664b 100644 --- a/lucene/core/src/java/org/apache/lucene/search/SortField.java +++ b/lucene/core/src/java/org/apache/lucene/search/SortField.java @@ -501,36 +501,44 @@ public Comparator getBytesComparator() { * @return {@link FieldComparator} to use when sorting */ public FieldComparator getComparator(final int numHits, final int sortPos) { - + final FieldComparator fieldComparator; switch (type) { case SCORE: - return new FieldComparator.RelevanceComparator(numHits); + fieldComparator = new FieldComparator.RelevanceComparator(numHits); + break; case DOC: - return new DocComparator(numHits, reverse, sortPos); + fieldComparator = new DocComparator(numHits, reverse, sortPos); + break; case INT: - return new IntComparator(numHits, field, (Integer) missingValue, reverse, sortPos); + fieldComparator = new IntComparator(numHits, field, (Integer) missingValue, reverse, sortPos); + break; case FLOAT: - return new FloatComparator(numHits, field, (Float) missingValue, reverse, sortPos); + fieldComparator = new FloatComparator(numHits, field, (Float) missingValue, reverse, sortPos); + break; case LONG: - return new LongComparator(numHits, field, (Long) missingValue, reverse, sortPos); + fieldComparator = new LongComparator(numHits, field, (Long) missingValue, reverse, sortPos); + break; case DOUBLE: - return new DoubleComparator(numHits, field, (Double) missingValue, reverse, sortPos); + fieldComparator = new DoubleComparator(numHits, field, (Double) missingValue, reverse, sortPos); + break; case CUSTOM: assert comparatorSource != null; - return comparatorSource.newComparator(field, numHits, sortPos, reverse); + fieldComparator = comparatorSource.newComparator(field, numHits, sortPos, reverse); + break; case STRING: return new FieldComparator.TermOrdValComparator( numHits, field, missingValue == STRING_LAST); case STRING_VAL: - return new FieldComparator.TermValComparator(numHits, field, missingValue == STRING_LAST); + fieldComparator = new FieldComparator.TermValComparator(numHits, field, missingValue == STRING_LAST); + break; case REWRITEABLE: throw new IllegalStateException( @@ -539,6 +547,10 @@ public FieldComparator getComparator(final int numHits, final int sortPos) { default: throw new IllegalStateException("Illegal sort type: " + type); } + if (getOptimizeSortWithPoints() == false) { + fieldComparator.disableSkipping(); + } + return fieldComparator; } /** diff --git a/lucene/core/src/java/org/apache/lucene/search/SortedNumericSortField.java b/lucene/core/src/java/org/apache/lucene/search/SortedNumericSortField.java index 45f318e55842..7d1706e09c38 100644 --- a/lucene/core/src/java/org/apache/lucene/search/SortedNumericSortField.java +++ b/lucene/core/src/java/org/apache/lucene/search/SortedNumericSortField.java @@ -242,9 +242,10 @@ public void setMissingValue(Object missingValue) { @Override public FieldComparator getComparator(int numHits, int sortPos) { + final FieldComparator fieldComparator; switch (type) { case INT: - return new IntComparator(numHits, getField(), (Integer) missingValue, reverse, sortPos) { + fieldComparator = new IntComparator(numHits, getField(), (Integer) missingValue, reverse, sortPos) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { @@ -258,8 +259,9 @@ protected NumericDocValues getNumericDocValues( }; } }; + break; case FLOAT: - return new FloatComparator(numHits, getField(), (Float) missingValue, reverse, sortPos) { + fieldComparator = new FloatComparator(numHits, getField(), (Float) missingValue, reverse, sortPos) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { @@ -273,8 +275,9 @@ protected NumericDocValues getNumericDocValues( }; } }; + break; case LONG: - return new LongComparator(numHits, getField(), (Long) missingValue, reverse, sortPos) { + fieldComparator = new LongComparator(numHits, getField(), (Long) missingValue, reverse, sortPos) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { @@ -288,8 +291,9 @@ protected NumericDocValues getNumericDocValues( }; } }; + break; case DOUBLE: - return new DoubleComparator(numHits, getField(), (Double) missingValue, reverse, sortPos) { + fieldComparator = new DoubleComparator(numHits, getField(), (Double) missingValue, reverse, sortPos) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { @@ -303,6 +307,7 @@ protected NumericDocValues getNumericDocValues( }; } }; + break; case CUSTOM: case DOC: case REWRITEABLE: @@ -312,6 +317,10 @@ protected NumericDocValues getNumericDocValues( default: throw new AssertionError(); } + if (getOptimizeSortWithPoints() == false) { + fieldComparator.disableSkipping(); + } + return fieldComparator; } private NumericDocValues getValue(LeafReader reader) throws IOException { From 6863d4a627d65f1bbb59d211c675091217e48d45 Mon Sep 17 00:00:00 2001 From: jimczi Date: Fri, 10 Sep 2021 17:55:59 +0200 Subject: [PATCH 2/2] apply spotless --- .../org/apache/lucene/search/SortField.java | 12 ++- .../lucene/search/SortedNumericSortField.java | 92 ++++++++++--------- 2 files changed, 56 insertions(+), 48 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 index 2daf59bb664b..18154136d2ff 100644 --- a/lucene/core/src/java/org/apache/lucene/search/SortField.java +++ b/lucene/core/src/java/org/apache/lucene/search/SortField.java @@ -512,11 +512,13 @@ public FieldComparator getComparator(final int numHits, final int sortPos) { break; case INT: - fieldComparator = new IntComparator(numHits, field, (Integer) missingValue, reverse, sortPos); + fieldComparator = + new IntComparator(numHits, field, (Integer) missingValue, reverse, sortPos); break; case FLOAT: - fieldComparator = new FloatComparator(numHits, field, (Float) missingValue, reverse, sortPos); + fieldComparator = + new FloatComparator(numHits, field, (Float) missingValue, reverse, sortPos); break; case LONG: @@ -524,7 +526,8 @@ public FieldComparator getComparator(final int numHits, final int sortPos) { break; case DOUBLE: - fieldComparator = new DoubleComparator(numHits, field, (Double) missingValue, reverse, sortPos); + fieldComparator = + new DoubleComparator(numHits, field, (Double) missingValue, reverse, sortPos); break; case CUSTOM: @@ -537,7 +540,8 @@ public FieldComparator getComparator(final int numHits, final int sortPos) { numHits, field, missingValue == STRING_LAST); case STRING_VAL: - fieldComparator = new FieldComparator.TermValComparator(numHits, field, missingValue == STRING_LAST); + fieldComparator = + new FieldComparator.TermValComparator(numHits, field, missingValue == STRING_LAST); break; case REWRITEABLE: diff --git a/lucene/core/src/java/org/apache/lucene/search/SortedNumericSortField.java b/lucene/core/src/java/org/apache/lucene/search/SortedNumericSortField.java index 7d1706e09c38..60ddaa9c03c4 100644 --- a/lucene/core/src/java/org/apache/lucene/search/SortedNumericSortField.java +++ b/lucene/core/src/java/org/apache/lucene/search/SortedNumericSortField.java @@ -245,68 +245,72 @@ public FieldComparator getComparator(int numHits, int sortPos) { final FieldComparator fieldComparator; switch (type) { case INT: - fieldComparator = new IntComparator(numHits, getField(), (Integer) missingValue, reverse, sortPos) { - @Override - public LeafFieldComparator getLeafComparator(LeafReaderContext context) - throws IOException { - return new IntLeafComparator(context) { + fieldComparator = + new IntComparator(numHits, getField(), (Integer) missingValue, reverse, sortPos) { @Override - protected NumericDocValues getNumericDocValues( - LeafReaderContext context, String field) throws IOException { - return SortedNumericSelector.wrap( - DocValues.getSortedNumeric(context.reader(), field), selector, type); + public LeafFieldComparator getLeafComparator(LeafReaderContext context) + throws IOException { + return new IntLeafComparator(context) { + @Override + protected NumericDocValues getNumericDocValues( + LeafReaderContext context, String field) throws IOException { + return SortedNumericSelector.wrap( + DocValues.getSortedNumeric(context.reader(), field), selector, type); + } + }; } }; - } - }; break; case FLOAT: - fieldComparator = new FloatComparator(numHits, getField(), (Float) missingValue, reverse, sortPos) { - @Override - public LeafFieldComparator getLeafComparator(LeafReaderContext context) - throws IOException { - return new FloatLeafComparator(context) { + fieldComparator = + new FloatComparator(numHits, getField(), (Float) missingValue, reverse, sortPos) { @Override - protected NumericDocValues getNumericDocValues( - LeafReaderContext context, String field) throws IOException { - return SortedNumericSelector.wrap( - DocValues.getSortedNumeric(context.reader(), field), selector, type); + public LeafFieldComparator getLeafComparator(LeafReaderContext context) + throws IOException { + return new FloatLeafComparator(context) { + @Override + protected NumericDocValues getNumericDocValues( + LeafReaderContext context, String field) throws IOException { + return SortedNumericSelector.wrap( + DocValues.getSortedNumeric(context.reader(), field), selector, type); + } + }; } }; - } - }; break; case LONG: - fieldComparator = new LongComparator(numHits, getField(), (Long) missingValue, reverse, sortPos) { - @Override - public LeafFieldComparator getLeafComparator(LeafReaderContext context) - throws IOException { - return new LongLeafComparator(context) { + fieldComparator = + new LongComparator(numHits, getField(), (Long) missingValue, reverse, sortPos) { @Override - protected NumericDocValues getNumericDocValues( - LeafReaderContext context, String field) throws IOException { - return SortedNumericSelector.wrap( - DocValues.getSortedNumeric(context.reader(), field), selector, type); + public LeafFieldComparator getLeafComparator(LeafReaderContext context) + throws IOException { + return new LongLeafComparator(context) { + @Override + protected NumericDocValues getNumericDocValues( + LeafReaderContext context, String field) throws IOException { + return SortedNumericSelector.wrap( + DocValues.getSortedNumeric(context.reader(), field), selector, type); + } + }; } }; - } - }; break; case DOUBLE: - fieldComparator = new DoubleComparator(numHits, getField(), (Double) missingValue, reverse, sortPos) { - @Override - public LeafFieldComparator getLeafComparator(LeafReaderContext context) - throws IOException { - return new DoubleLeafComparator(context) { + fieldComparator = + new DoubleComparator(numHits, getField(), (Double) missingValue, reverse, sortPos) { @Override - protected NumericDocValues getNumericDocValues( - LeafReaderContext context, String field) throws IOException { - return SortedNumericSelector.wrap( - DocValues.getSortedNumeric(context.reader(), field), selector, type); + public LeafFieldComparator getLeafComparator(LeafReaderContext context) + throws IOException { + return new DoubleLeafComparator(context) { + @Override + protected NumericDocValues getNumericDocValues( + LeafReaderContext context, String field) throws IOException { + return SortedNumericSelector.wrap( + DocValues.getSortedNumeric(context.reader(), field), selector, type); + } + }; } }; - } - }; break; case CUSTOM: case DOC: