From 19537578dd4d4974237fc9f911656d3c996f08ec Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 13 Sep 2021 07:31:43 +0200 Subject: [PATCH] LUCENE-10089: Disable numeric sort optimization early (#291) 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 | 34 ++++-- .../lucene/search/SortedNumericSortField.java | 101 ++++++++++-------- 3 files changed, 82 insertions(+), 54 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..18154136d2ff 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,48 @@ 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 +551,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..60ddaa9c03c4 100644 --- a/lucene/core/src/java/org/apache/lucene/search/SortedNumericSortField.java +++ b/lucene/core/src/java/org/apache/lucene/search/SortedNumericSortField.java @@ -242,67 +242,76 @@ 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) { - @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: - return 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: - return 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: - return 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: case REWRITEABLE: @@ -312,6 +321,10 @@ protected NumericDocValues getNumericDocValues( default: throw new AssertionError(); } + if (getOptimizeSortWithPoints() == false) { + fieldComparator.disableSkipping(); + } + return fieldComparator; } private NumericDocValues getValue(LeafReader reader) throws IOException {