From 99d1db6a18326c5f982854765207a117517bece1 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 29 Oct 2025 09:45:50 +0000 Subject: [PATCH 01/23] WIP: pluggable competitiveDISIBuilder and secondary sort-based iterator --- .../LongValuesComparatorSource.java | 41 +- .../SecondarySortIterator.java | 168 ++++++ .../lucene/comparators/XLongComparator.java | 115 ++++ .../comparators/XNumericComparator.java | 514 ++++++++++++++++++ .../XUpdateableDocIdSetIterator.java | 73 +++ 5 files changed, 909 insertions(+), 2 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java create mode 100644 server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java create mode 100644 server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java create mode 100644 server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index d27c1b4784311..4c419d9563617 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java @@ -8,6 +8,7 @@ */ package org.elasticsearch.index.fielddata.fieldcomparator; +import org.apache.lucene.index.DocValuesSkipper; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; import org.apache.lucene.search.DocIdSetIterator; @@ -15,8 +16,8 @@ import org.apache.lucene.search.LeafFieldComparator; import org.apache.lucene.search.LongValues; import org.apache.lucene.search.Pruning; +import org.apache.lucene.search.Sort; import org.apache.lucene.search.SortField; -import org.apache.lucene.search.comparators.LongComparator; import org.apache.lucene.util.BitSet; import org.elasticsearch.common.time.DateUtils; import org.elasticsearch.common.util.BigArrays; @@ -28,6 +29,8 @@ import org.elasticsearch.index.fielddata.LeafNumericFieldData; import org.elasticsearch.index.fielddata.SortedNumericLongValues; import org.elasticsearch.index.fielddata.plain.SortedNumericIndexFieldData; +import org.elasticsearch.lucene.comparators.XLongComparator; +import org.elasticsearch.lucene.comparators.XNumericComparator; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.MultiValueMode; import org.elasticsearch.search.sort.BucketedSort; @@ -103,7 +106,7 @@ public FieldComparator newComparator(String fieldname, int numHits, Pruning e final long lMissingValue = (Long) missingObject(missingValue, reversed); // NOTE: it's important to pass null as a missing value in the constructor so that // the comparator doesn't check docsWithField since we replace missing values in select() - return new LongComparator(numHits, null, null, reversed, Pruning.NONE) { + return new XLongComparator(numHits, null, null, reversed, Pruning.NONE) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new LongLeafComparator(context) { @@ -111,6 +114,40 @@ public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws I protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) throws IOException { return wrap(getLongValues(context, lMissingValue)); } + + @Override + protected XNumericComparator.CompetitiveDISIBuilder buildCompetitiveDISIBuilder(LeafReaderContext context) + throws IOException { + Sort indexSort = context.reader().getMetaData().sort(); + if (indexSort == null) { + return super.buildCompetitiveDISIBuilder(context); + } + SortField[] sortFields = indexSort.getSort(); + if (sortFields.length != 2) { + return super.buildCompetitiveDISIBuilder(context); + } + if (sortFields[1].getField().equals(field) == false) { + return super.buildCompetitiveDISIBuilder(context); + } + DocValuesSkipper skipper = context.reader().getDocValuesSkipper(field); + DocValuesSkipper primaryFieldSkipper = context.reader().getDocValuesSkipper(sortFields[0].getField()); + if (primaryFieldSkipper == null) { + return super.buildCompetitiveDISIBuilder(context); + } + return new CompetitiveDISIBuilder(this) { + @Override + protected int docCount() { + return skipper.docCount(); + } + + @Override + protected void doUpdateCompetitiveIterator() { + competitiveIterator.update( + new SecondarySortIterator(docValues, skipper, primaryFieldSkipper, minValueAsLong, maxValueAsLong) + ); + } + }; + } }; } }; diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java new file mode 100644 index 0000000000000..c33a7bef3515e --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java @@ -0,0 +1,168 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.index.fielddata.fieldcomparator; + +import org.apache.lucene.index.DocValuesSkipper; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.search.DocIdSetIterator; + +import java.io.IOException; + +/** + * A competitive DocIdSetIterator that examines the values of a secondary + * sort field and tries to exclude documents with values outside a given + * range, using DocValueSkippers on the primary sort field to advance rapidly + * to the next block of values. + */ +class SecondarySortIterator extends DocIdSetIterator { + + final NumericDocValues values; + + final DocValuesSkipper valueSkipper; + final DocValuesSkipper primaryFieldSkipper; + final long minValue; + final long maxValue; + + int docID = -1; + boolean skipperMatch; + int primaryFieldUpTo = -1; + int valueFieldUpTo = -1; + + SecondarySortIterator( + NumericDocValues values, + DocValuesSkipper valueSkipper, + DocValuesSkipper primaryFieldSkipper, + long minValue, + long maxValue + ) { + this.values = values; + this.valueSkipper = valueSkipper; + this.primaryFieldSkipper = primaryFieldSkipper; + this.minValue = minValue; + this.maxValue = maxValue; + + valueFieldUpTo = valueSkipper.maxDocID(0); + primaryFieldUpTo = primaryFieldSkipper.maxDocID(0); + } + + @Override + public int docID() { + return docID; + } + + @Override + public int nextDoc() throws IOException { + return advance(docID + 1); + } + + @Override + public int advance(int target) throws IOException { + skipperMatch = false; + target = values.advance(target); + if (target == DocIdSetIterator.NO_MORE_DOCS) { + return docID = target; + } + while (true) { + if (target > valueFieldUpTo) { + valueSkipper.advance(target); + valueFieldUpTo = valueSkipper.maxDocID(0); + long minValue = valueSkipper.minValue(0); + long maxValue = valueSkipper.maxValue(0); + if (minValue > this.maxValue || maxValue < this.minValue) { + // outside the desired range, skip forward + for (int level = 1; level < valueSkipper.numLevels(); level++) { + minValue = valueSkipper.minValue(level); + maxValue = valueSkipper.maxValue(level); + if (minValue > this.maxValue || maxValue < this.minValue) { + valueFieldUpTo = valueSkipper.maxDocID(level); + } else { + break; + } + } + + int upTo = valueFieldUpTo; + if (maxValue < this.minValue) { + // We've moved past the end of the valid values in the secondary sort field + // for this primary value. Advance the primary skipper to find the starting point + // for the next primary value, where the secondary field values will have reset + primaryFieldSkipper.advance(target); + primaryFieldUpTo = primaryFieldSkipper.maxDocID(0); + if (primaryFieldSkipper.minValue(0) == primaryFieldSkipper.maxValue(0)) { + for (int level = 1; level < primaryFieldSkipper.numLevels(); level++) { + if (primaryFieldSkipper.minValue(level) == primaryFieldSkipper.maxValue(level)) { + primaryFieldUpTo = primaryFieldSkipper.maxDocID(level); + } else { + break; + } + } + } + if (primaryFieldUpTo > upTo) { + upTo = primaryFieldUpTo; + } + } + + target = values.advance(upTo + 1); + if (target == DocIdSetIterator.NO_MORE_DOCS) { + return docID = target; + } + } else if (minValue >= this.minValue && maxValue <= this.maxValue) { + assert valueSkipper.docCount(0) == valueSkipper.maxDocID(0) - valueSkipper.minDocID(0) + 1; + skipperMatch = true; + return docID = target; + } + } + + long value = values.longValue(); + if (value < minValue && target > primaryFieldUpTo) { + primaryFieldSkipper.advance(target); + primaryFieldUpTo = primaryFieldSkipper.maxDocID(0); + if (primaryFieldSkipper.minValue(0) == primaryFieldSkipper.maxValue(0)) { + for (int level = 1; level < primaryFieldSkipper.numLevels(); level++) { + if (primaryFieldSkipper.minValue(level) == primaryFieldSkipper.maxValue(level)) { + primaryFieldUpTo = primaryFieldSkipper.maxDocID(level); + } else { + break; + } + } + target = values.advance(primaryFieldUpTo + 1); + if (target == DocIdSetIterator.NO_MORE_DOCS) { + return docID = target; + } + } else { + target = values.nextDoc(); + if (target == DocIdSetIterator.NO_MORE_DOCS) { + return docID = target; + } + } + } else if (value >= minValue && value <= maxValue) { + return docID = target; + } else { + target = values.nextDoc(); + if (target == DocIdSetIterator.NO_MORE_DOCS) { + return docID = target; + } + } + } + } + + @Override + public int docIDRunEnd() throws IOException { + if (skipperMatch) { + return valueFieldUpTo + 1; + } + return super.docIDRunEnd(); + } + + @Override + public long cost() { + return values.cost(); + } + +} diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java new file mode 100644 index 0000000000000..6caa94382a6bd --- /dev/null +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java @@ -0,0 +1,115 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.lucene.comparators; + +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.LeafFieldComparator; +import org.apache.lucene.search.Pruning; +import org.apache.lucene.search.comparators.LongComparator; +import org.apache.lucene.util.NumericUtils; + +import java.io.IOException; + +public class XLongComparator extends XNumericComparator { + + private final long[] values; + protected long topValue; + protected long bottom; + + public XLongComparator( + int numHits, String field, Long missingValue, boolean reverse, Pruning pruning) { + super(field, missingValue != null ? missingValue : 0L, reverse, pruning, Long.BYTES); + values = new long[numHits]; + } + + @Override + public int compare(int slot1, int slot2) { + return Long.compare(values[slot1], values[slot2]); + } + + @Override + public void setTopValue(Long value) { + super.setTopValue(value); + topValue = value; + } + + @Override + public Long value(int slot) { + return Long.valueOf(values[slot]); + } + + @Override + protected long missingValueAsComparableLong() { + return missingValue; + } + + @Override + protected long sortableBytesToLong(byte[] bytes) { + return NumericUtils.sortableBytesToLong(bytes, 0); + } + + @Override + public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { + return new LongLeafComparator(context); + } + + /** Leaf comparator for {@link LongComparator} that provides skipping functionality */ + public class LongLeafComparator extends NumericLeafComparator { + + public LongLeafComparator(LeafReaderContext context) throws IOException { + super(context); + } + + @Override + protected XNumericComparator.CompetitiveDISIBuilder buildCompetitiveDISIBuilder(LeafReaderContext context) throws IOException { + return super.buildCompetitiveDISIBuilder(context); + } + + private long getValueForDoc(int doc) throws IOException { + if (docValues.advanceExact(doc)) { + return docValues.longValue(); + } else { + return missingValue; + } + } + + @Override + public void setBottom(int slot) throws IOException { + bottom = values[slot]; + super.setBottom(slot); + } + + @Override + public int compareBottom(int doc) throws IOException { + return Long.compare(bottom, getValueForDoc(doc)); + } + + @Override + public int compareTop(int doc) throws IOException { + return Long.compare(topValue, getValueForDoc(doc)); + } + + @Override + public void copy(int slot, int doc) throws IOException { + values[slot] = getValueForDoc(doc); + super.copy(slot, doc); + } + + @Override + protected long bottomAsComparableLong() { + return bottom; + } + + @Override + protected long topAsComparableLong() { + return topValue; + } + } +} diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java new file mode 100644 index 0000000000000..84cab89743351 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java @@ -0,0 +1,514 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.lucene.comparators; + +import org.apache.lucene.index.DocValues; +import org.apache.lucene.index.DocValuesSkipper; +import org.apache.lucene.index.FieldInfo; +import org.apache.lucene.index.LeafReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.PointValues; +import org.apache.lucene.search.DocIdSetIterator; +import org.apache.lucene.search.DocValuesRangeIterator; +import org.apache.lucene.search.FieldComparator; +import org.apache.lucene.search.LeafFieldComparator; +import org.apache.lucene.search.Pruning; +import org.apache.lucene.search.Scorable; +import org.apache.lucene.search.Scorer; +import org.apache.lucene.search.TwoPhaseIterator; +import org.apache.lucene.util.DocIdSetBuilder; +import org.apache.lucene.util.IntsRef; + +import java.io.IOException; + +/** + * Abstract numeric comparator for comparing numeric values. This comparator provides a skipping + * functionality – an iterator that can skip over non-competitive documents. + * + *

Parameter {@code field} provided in the constructor is used as a field name in the default + * implementations of the methods {@code getNumericDocValues} and {@code getPointValues} to retrieve + * doc values and points. You can pass a dummy value for a field name (e.g. when sorting by script), + * but in this case you must override both of these methods. + */ +public abstract class XNumericComparator extends FieldComparator { + + // MIN_SKIP_INTERVAL and MAX_SKIP_INTERVAL both should be powers of 2 + private static final int MIN_SKIP_INTERVAL = 32; + private static final int MAX_SKIP_INTERVAL = 8192; + protected final T missingValue; + private final long missingValueAsLong; + protected final String field; + protected final boolean reverse; + private final int bytesCount; // how many bytes are used to encode this number + + protected boolean topValueSet; + protected boolean singleSort; // singleSort is true, if sort is based on a single sort field. + protected boolean hitsThresholdReached; + protected boolean queueFull; + protected Pruning pruning; + + protected XNumericComparator( + String field, T missingValue, boolean reverse, Pruning pruning, int bytesCount) { + this.field = field; + this.missingValue = missingValue; + this.missingValueAsLong = missingValueAsComparableLong(); + this.reverse = reverse; + this.pruning = pruning; + this.bytesCount = bytesCount; + } + + @Override + public void setTopValue(T value) { + topValueSet = true; + } + + @Override + public void setSingleSort() { + singleSort = true; + } + + @Override + public void disableSkipping() { + pruning = Pruning.NONE; + } + + protected abstract long missingValueAsComparableLong(); + + /** + * Decode sortable bytes to long. It should be consistent with the codec that {@link PointValues} + * of this field is using. + */ + protected abstract long sortableBytesToLong(byte[] bytes); + + /** Leaf comparator for {@link org.apache.lucene.search.comparators.NumericComparator} that provides skipping functionality */ + public abstract class NumericLeafComparator implements LeafFieldComparator { + private final LeafReaderContext context; + protected final NumericDocValues docValues; + private final CompetitiveDISIBuilder competitiveDISIBuilder; + + public NumericLeafComparator(LeafReaderContext context) throws IOException { + this.context = context; + this.docValues = getNumericDocValues(context, field); + competitiveDISIBuilder = buildCompetitiveDISIBuilder(context); + } + + protected CompetitiveDISIBuilder buildCompetitiveDISIBuilder(LeafReaderContext context) throws IOException { + if (pruning == Pruning.NONE) { + return null; + } + LeafReader reader = context.reader(); + PointValues pointValues = reader.getPointValues(field); + if (pointValues != null) { + return new PointsCompetitiveDISIBuilder(pointValues, this); + } else { + DocValuesSkipper skipper = reader.getDocValuesSkipper(field); + if (skipper != null) { + return new DVSkipperCompetitiveDISIBuilder(skipper, this); + } + } + return null; + } + + /** + * Retrieves the NumericDocValues for the field in this segment + * + *

If you override this method, you should probably always disable skipping as the comparator + * uses values from the points index to build its competitive iterators, and assumes that the + * values in doc values and points are the same. + * + * @param context – reader context + * @param field - field name + * @return numeric doc values for the field in this segment. + * @throws IOException If there is a low-level I/O error + */ + protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) + throws IOException { + return DocValues.getNumeric(context.reader(), field); + } + + @Override + public void setBottom(int slot) throws IOException { + queueFull = true; // if we are setting bottom, it means that we have collected enough hits + if (competitiveDISIBuilder != null) { + competitiveDISIBuilder.updateCompetitiveIterator(); + } + } + + @Override + public void copy(int slot, int doc) throws IOException { + if (competitiveDISIBuilder != null) { + competitiveDISIBuilder.setMaxDocVisited(doc); + } + } + + @Override + public void setScorer(Scorable scorer) throws IOException { + if (competitiveDISIBuilder != null) { + competitiveDISIBuilder.setScorer(scorer); + } + } + + @Override + public void setHitsThresholdReached() throws IOException { + hitsThresholdReached = true; + if (competitiveDISIBuilder != null) { + competitiveDISIBuilder.updateCompetitiveIterator(); + } + } + + @Override + public DocIdSetIterator competitiveIterator() { + return competitiveDISIBuilder == null ? null : competitiveDISIBuilder.competitiveIterator; + } + + protected abstract long bottomAsComparableLong(); + + protected abstract long topAsComparableLong(); + } + + protected abstract class CompetitiveDISIBuilder { + + final int maxDoc; + final NumericLeafComparator leafComparator; + + /** According to {@link FieldComparator#setTopValue}, topValueSet is final in leafComparator */ + final boolean leafTopSet = topValueSet; + + protected final XUpdateableDocIdSetIterator competitiveIterator = new XUpdateableDocIdSetIterator(); + protected long minValueAsLong = Long.MIN_VALUE; + protected long maxValueAsLong = Long.MAX_VALUE; + int maxDocVisited = -1; + int updateCounter = 0; + int currentSkipInterval = MIN_SKIP_INTERVAL; + + protected CompetitiveDISIBuilder(NumericLeafComparator leafComparator) { + this.leafComparator = leafComparator; + this.maxDoc = leafComparator.context.reader().maxDoc(); + this.competitiveIterator.update(DocIdSetIterator.all(maxDoc)); + if (leafTopSet) { + encodeTop(); + } + } + + void setScorer(Scorable scorer) throws IOException {} + + protected abstract int docCount(); + + final void updateCompetitiveIterator() throws IOException { + if (hitsThresholdReached == false) { + return; + } + if (leafTopSet == false && queueFull == false) { + return; + } + + // if some documents have missing points, check that missing values prohibits optimization + if (docCount() < maxDoc && isMissingValueCompetitive()) { + return; + } + + updateCounter++; + // Start sampling if we get called too much + if (updateCounter > 256 + && (updateCounter & (currentSkipInterval - 1)) != currentSkipInterval - 1) { + return; + } + + if (queueFull) { + encodeBottom(); + } + + doUpdateCompetitiveIterator(); + } + + protected abstract void doUpdateCompetitiveIterator() throws IOException; + + private void setMaxDocVisited(int maxDocVisited) { + this.maxDocVisited = maxDocVisited; + } + + /** + * If {@link XNumericComparator#pruning} equals {@link Pruning#GREATER_THAN_OR_EQUAL_TO}, we + * could better tune the {@link #maxValueAsLong}/{@link #minValueAsLong}. For instance, if the + * sort is ascending and bottom value is 5, we will use a range on [MIN_VALUE, 4]. + */ + private void encodeBottom() { + if (reverse == false) { + maxValueAsLong = leafComparator.bottomAsComparableLong(); + if (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO && maxValueAsLong != Long.MIN_VALUE) { + maxValueAsLong--; + } + } else { + minValueAsLong = leafComparator.bottomAsComparableLong(); + if (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO && minValueAsLong != Long.MAX_VALUE) { + minValueAsLong++; + } + } + } + + /** + * If {@link XNumericComparator#pruning} equals {@link Pruning#GREATER_THAN_OR_EQUAL_TO}, we + * could better tune the {@link #minValueAsLong}/{@link #minValueAsLong}. For instance, if the + * sort is ascending and top value is 3, we will use a range on [4, MAX_VALUE]. + */ + private void encodeTop() { + if (reverse == false) { + minValueAsLong = leafComparator.topAsComparableLong(); + if (singleSort + && pruning == Pruning.GREATER_THAN_OR_EQUAL_TO + && queueFull + && minValueAsLong != Long.MAX_VALUE) { + minValueAsLong++; + } + } else { + maxValueAsLong = leafComparator.topAsComparableLong(); + if (singleSort + && pruning == Pruning.GREATER_THAN_OR_EQUAL_TO + && queueFull + && maxValueAsLong != Long.MIN_VALUE) { + maxValueAsLong--; + } + } + } + + boolean isMissingValueCompetitive() { + // if queue is full, compare with bottom first, + // if competitive, then check if we can compare with topValue + if (queueFull) { + int result = Long.compare(missingValueAsLong, leafComparator.bottomAsComparableLong()); + // in reverse (desc) sort missingValue is competitive when it's greater or equal to bottom, + // in asc sort missingValue is competitive when it's smaller or equal to bottom + final boolean competitive = + reverse + ? (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO ? result > 0 : result >= 0) + : (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO ? result < 0 : result <= 0); + if (competitive == false) { + return false; + } + } + + if (leafTopSet) { + int result = Long.compare(missingValueAsLong, leafComparator.topAsComparableLong()); + // in reverse (desc) sort missingValue is competitive when it's smaller or equal to + // topValue, + // in asc sort missingValue is competitive when it's greater or equal to topValue + return reverse ? (result <= 0) : (result >= 0); + } + + // by default competitive + return true; + } + } + + private class PointsCompetitiveDISIBuilder extends CompetitiveDISIBuilder { + + private final PointValues pointValues; + // lazily constructed to avoid performance overhead when this is not used + private PointValues.PointTree pointTree; + private long iteratorCost = -1; + // helps to be conservative about increasing the sampling interval + private int tryUpdateFailCount = 0; + + @SuppressWarnings("checkstyle:RedundantModifier") + public PointsCompetitiveDISIBuilder(PointValues pointValues, NumericLeafComparator comparator) { + super(comparator); + LeafReaderContext context = comparator.context; + 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 " + + this + + " expected " + + bytesCount); + } + this.pointValues = pointValues; + } + + @Override + void setScorer(Scorable scorer) throws IOException { + if (iteratorCost == -1) { + if (scorer instanceof Scorer) { + iteratorCost = + ((Scorer) scorer).iterator().cost(); // starting iterator cost is the scorer's cost + } else { + iteratorCost = maxDoc; + } + updateCompetitiveIterator(); // update an iterator when we have a new segment + } + } + + @Override + protected int docCount() { + return pointValues.getDocCount(); + } + + @Override + protected void doUpdateCompetitiveIterator() throws IOException { + DocIdSetBuilder result = new DocIdSetBuilder(maxDoc); + PointValues.IntersectVisitor visitor = + new PointValues.IntersectVisitor() { + DocIdSetBuilder.BulkAdder adder; + + @Override + public void grow(int count) { + adder = result.grow(count); + } + + @Override + public void visit(int docID) { + if (docID <= maxDocVisited) { + return; // Already visited or skipped + } + adder.add(docID); + } + + @Override + public void visit(int docID, byte[] packedValue) { + if (docID <= maxDocVisited) { + return; // already visited or skipped + } + long l = sortableBytesToLong(packedValue); + if (l >= minValueAsLong && l <= maxValueAsLong) { + adder.add(docID); // doc is competitive + } + } + + @Override + public void visit(DocIdSetIterator iterator) throws IOException { + if (iterator.advance(maxDocVisited + 1) != DocIdSetIterator.NO_MORE_DOCS) { + adder.add(iterator.docID()); + adder.add(iterator); + } + } + + @Override + public void visit(IntsRef ref) { + adder.add(ref, maxDocVisited + 1); + } + + @Override + public PointValues.Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { + long min = sortableBytesToLong(minPackedValue); + long max = sortableBytesToLong(maxPackedValue); + + if (min > maxValueAsLong || max < minValueAsLong) { + // 1. cmp ==0 and pruning==Pruning.GREATER_THAN_OR_EQUAL_TO : if the sort is + // ascending then maxValueAsLong is bottom's next less value, so it is competitive + // 2. cmp ==0 and pruning==Pruning.GREATER_THAN: maxValueAsLong equals to + // bottom, but there are multiple comparators, so it could be competitive + return PointValues.Relation.CELL_OUTSIDE_QUERY; + } + + if (min < minValueAsLong || max > maxValueAsLong) { + return PointValues.Relation.CELL_CROSSES_QUERY; + } + return PointValues.Relation.CELL_INSIDE_QUERY; + } + }; + + final long threshold = iteratorCost >>> 3; + + if (PointValues.isEstimatedPointCountGreaterThanOrEqualTo( + visitor, getPointTree(), threshold)) { + // the new range is not selective enough to be worth materializing, it doesn't reduce number + // of docs at least 8x + updateSkipInterval(false); + if (pointValues.getDocCount() < iteratorCost) { + // Use the set of doc with values to help drive iteration + competitiveIterator.update( + leafComparator.getNumericDocValues(leafComparator.context, field)); + iteratorCost = pointValues.getDocCount(); + } + return; + } + pointValues.intersect(visitor); + competitiveIterator.update(result.build().iterator()); + iteratorCost = competitiveIterator.cost(); + updateSkipInterval(true); + } + + private PointValues.PointTree getPointTree() throws IOException { + if (pointTree == null) { + pointTree = pointValues.getPointTree(); + } + return pointTree; + } + + private void updateSkipInterval(boolean success) { + if (updateCounter > 256) { + if (success) { + currentSkipInterval = Math.max(currentSkipInterval / 2, MIN_SKIP_INTERVAL); + tryUpdateFailCount = 0; + } else { + if (tryUpdateFailCount >= 3) { + currentSkipInterval = Math.min(currentSkipInterval * 2, MAX_SKIP_INTERVAL); + tryUpdateFailCount = 0; + } else { + tryUpdateFailCount++; + } + } + } + } + } + + private class DVSkipperCompetitiveDISIBuilder extends CompetitiveDISIBuilder { + + private final DocValuesSkipper skipper; + private final TwoPhaseIterator innerTwoPhase; + + @SuppressWarnings("checkstyle:RedundantModifier") + public DVSkipperCompetitiveDISIBuilder( + DocValuesSkipper skipper, NumericLeafComparator leafComparator) throws IOException { + super(leafComparator); + this.skipper = skipper; + NumericDocValues docValues = + leafComparator.getNumericDocValues(leafComparator.context, field); + innerTwoPhase = + new TwoPhaseIterator(docValues) { + @Override + public boolean matches() throws IOException { + final long value = docValues.longValue(); + return value >= minValueAsLong && value <= maxValueAsLong; + } + + @Override + public float matchCost() { + return 2; // 2 comparisons + } + }; + } + + @Override + protected int docCount() { + return skipper.docCount(); + } + + @Override + protected void doUpdateCompetitiveIterator() { + TwoPhaseIterator twoPhaseIterator = + new DocValuesRangeIterator(innerTwoPhase, skipper, minValueAsLong, maxValueAsLong, false); + competitiveIterator.update(TwoPhaseIterator.asDocIdSetIterator(twoPhaseIterator)); + } + } +} + diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java new file mode 100644 index 0000000000000..2f9c5767b1a57 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java @@ -0,0 +1,73 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.lucene.comparators; + +import org.apache.lucene.search.AbstractDocIdSetIterator; +import org.apache.lucene.search.DocIdSetIterator; +import org.apache.lucene.util.FixedBitSet; + +import java.io.IOException; +import java.util.Objects; + +public final class XUpdateableDocIdSetIterator extends AbstractDocIdSetIterator { + + private DocIdSetIterator in = DocIdSetIterator.empty(); + + /** + * Update the wrapped {@link DocIdSetIterator}. It doesn't need to be positioned on the same doc + * ID as this iterator. + */ + public void update(DocIdSetIterator iterator) { + this.in = Objects.requireNonNull(iterator); + } + + @Override + public int nextDoc() throws IOException { + return advance(doc + 1); + } + + @Override + public int advance(int target) throws IOException { + int curDoc = in.docID(); + if (curDoc < target) { + curDoc = in.advance(target); + } + return this.doc = curDoc; + } + + @Override + public long cost() { + return in.cost(); + } + + @Override + public void intoBitSet(int upTo, FixedBitSet bitSet, int offset) throws IOException { + // #update may have been just called + if (in.docID() < doc) { + in.advance(doc); + } + in.intoBitSet(upTo, bitSet, offset); + doc = in.docID(); + } + + @Override + public int docIDRunEnd() throws IOException { + // #update may have been just called + if (in.docID() < doc) { + in.advance(doc); + } + if (in.docID() == doc) { + return in.docIDRunEnd(); + } else { + return super.docIDRunEnd(); + } + } +} + From 45882c8e2415a4cbc708bf5f0589b97a40b8411e Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 29 Oct 2025 14:33:17 +0000 Subject: [PATCH 02/23] wip --- .../elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java b/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java index 75f7a41db9d26..d81b5920dc57d 100644 --- a/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java +++ b/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java @@ -17,6 +17,7 @@ import org.elasticsearch.action.datastreams.CreateDataStreamAction; import org.elasticsearch.action.datastreams.GetDataStreamAction; import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; import org.elasticsearch.cluster.metadata.Template; @@ -28,6 +29,7 @@ import org.elasticsearch.index.IndexService; import org.elasticsearch.index.IndexSortConfig; import org.elasticsearch.index.engine.Engine; +import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.license.LicenseSettings; @@ -126,6 +128,13 @@ public void testHostnameTimestampSortConfig() throws IOException { } assertOrder(backingIndex, orderedDocs); + + + SearchRequest searchRequest = new SearchRequest(dataStreamName); + searchRequest.source().sort("@timestamp", SortOrder.DESC).query( + new TermQueryBuilder("host.name", "aaa") + ); + client().search(searchRequest); } public void testTimestampOnlySortConfig() throws IOException { From f72677c8a29f69aa858efe41720a51cea2e2120d Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 30 Oct 2025 12:13:54 +0000 Subject: [PATCH 03/23] Force use of competitive comparators --- .../fielddata/IndexNumericFieldData.java | 30 ++++++++++++++----- .../LongValuesComparatorSource.java | 2 +- .../search/sort/FieldSortBuilderTests.java | 8 +++++ .../xpack/logsdb/LogsdbSortConfigIT.java | 7 +++-- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java index 53a8bcf2776aa..f6e03c40db94f 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java @@ -9,6 +9,8 @@ package org.elasticsearch.index.fielddata; +import org.apache.lucene.search.FieldComparator; +import org.apache.lucene.search.Pruning; import org.apache.lucene.search.SortField; import org.apache.lucene.search.SortedNumericSelector; import org.apache.lucene.search.SortedNumericSortField; @@ -104,18 +106,26 @@ public final SortField sortField( boolean requiresCustomComparator = nested != null || (sortMode != MultiValueMode.MAX && sortMode != MultiValueMode.MIN) || targetNumericType != getNumericType(); + boolean canUseOptimizedSort = canUseOptimizedSort(indexType()); if (sortRequiresCustomComparator() || requiresCustomComparator) { - SortField sortField = new SortField(getFieldName(), source, reverse); - sortField.setOptimizeSortWithPoints(requiresCustomComparator == false && canUseOptimizedSort(indexType())); - return sortField; + return new SortField(getFieldName(), source, reverse) { + @Override + public FieldComparator getComparator(int numHits, Pruning pruning) { + return super.getComparator(numHits, canUseOptimizedSort == false || requiresCustomComparator ? Pruning.NONE : pruning); + } + }; } SortedNumericSelector.Type selectorType = sortMode == MultiValueMode.MAX ? SortedNumericSelector.Type.MAX : SortedNumericSelector.Type.MIN; - SortField sortField = new SortedNumericSortField(getFieldName(), getNumericType().sortFieldType, reverse, selectorType); + SortField sortField = new SortedNumericSortField(getFieldName(), getNumericType().sortFieldType, reverse, selectorType) { + @Override + public FieldComparator getComparator(int numHits, Pruning pruning) { + return source.newComparator(getFieldName(), numHits, canUseOptimizedSort == false ? Pruning.NONE : pruning, reverse); + } + }; sortField.setMissingValue(source.missingObject(missingValue, reverse)); - sortField.setOptimizeSortWithPoints(canUseOptimizedSort(indexType())); return sortField; } @@ -182,11 +192,15 @@ public SortField sortField( SortField.Type.LONG, sortField.getReverse(), numericSortField.getSelector() - ); + ) { + @Override + public FieldComparator getComparator(int numHits, Pruning pruning) { + // we don't optimize sorting on int field for old indices + return super.getComparator(numHits, Pruning.NONE); + } + }; XFieldComparatorSource longSource = comparatorSource(NumericType.LONG, missingValue, sortMode, nested); rewrittenSortField.setMissingValue(longSource.missingObject(missingValue, reverse)); - // we don't optimize sorting on int field for old indices - rewrittenSortField.setOptimizeSortWithPoints(false); return rewrittenSortField; } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index 4c419d9563617..c1ad163ebb03d 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java @@ -106,7 +106,7 @@ public FieldComparator newComparator(String fieldname, int numHits, Pruning e final long lMissingValue = (Long) missingObject(missingValue, reversed); // NOTE: it's important to pass null as a missing value in the constructor so that // the comparator doesn't check docsWithField since we replace missing values in select() - return new XLongComparator(numHits, null, null, reversed, Pruning.NONE) { + return new XLongComparator(numHits, fieldname, null, reversed, Pruning.NONE) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new LongLeafComparator(context) { diff --git a/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java b/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java index 521f6e9b5eb2f..f79d02b8c29af 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java @@ -175,6 +175,14 @@ public void testBuildSortFieldMissingValue() throws IOException { assertEquals(expectedSortField, sortField); } + // Specialised assertEquals to handle subclasses of SortedNumericSortField + public static void assertEquals(SortedNumericSortField expected, SortField actual) { + assertEquals(expected.getField(), actual.getField()); + assertEquals(expected.getMissingValue(), actual.getMissingValue()); + assertEquals(expected.getType(), actual.getType()); + assertEquals(expected.getReverse(), actual.getReverse()); + } + /** * Test that the sort builder order gets transferred correctly to the SortField */ diff --git a/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java b/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java index d81b5920dc57d..c2e6a5ab5e327 100644 --- a/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java +++ b/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java @@ -78,7 +78,7 @@ private DocWithId doc(String source) { return new DocWithId(Integer.toString(id++), source); } - public void testHostnameTimestampSortConfig() throws IOException { + public void testHostnameTimestampSortConfig() throws Exception { final String dataStreamName = "test-logsdb-sort-hostname-timestamp"; final String MAPPING = """ @@ -129,12 +129,13 @@ public void testHostnameTimestampSortConfig() throws IOException { assertOrder(backingIndex, orderedDocs); - SearchRequest searchRequest = new SearchRequest(dataStreamName); searchRequest.source().sort("@timestamp", SortOrder.DESC).query( new TermQueryBuilder("host.name", "aaa") ); - client().search(searchRequest); + var response = client().search(searchRequest).get(); + assertEquals(4, response.getHits().getHits().length); + response.decRef(); } public void testTimestampOnlySortConfig() throws IOException { From 4e704f1a5a5fb80f95973bcedbce554d8c161c65 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 30 Oct 2025 13:50:06 +0000 Subject: [PATCH 04/23] tests --- .../fieldcomparator/LongValuesComparatorSource.java | 2 +- .../org/elasticsearch/index/mapper/NumberFieldTypeTests.java | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index c1ad163ebb03d..465a69503abec 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java @@ -106,7 +106,7 @@ public FieldComparator newComparator(String fieldname, int numHits, Pruning e final long lMissingValue = (Long) missingObject(missingValue, reversed); // NOTE: it's important to pass null as a missing value in the constructor so that // the comparator doesn't check docsWithField since we replace missing values in select() - return new XLongComparator(numHits, fieldname, null, reversed, Pruning.NONE) { + return new XLongComparator(numHits, fieldname, null, reversed, enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new LongLeafComparator(context) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java index 2b94f469e6c18..48019f2de4adc 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -25,6 +25,7 @@ import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.IndexSortSortedNumericDocValuesRangeQuery; import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.Pruning; import org.apache.lucene.search.Query; import org.apache.lucene.search.Sort; import org.apache.lucene.search.SortField; @@ -767,6 +768,10 @@ public void doTestIndexSortRangeQueries(NumberType type, Supplier valueS } } + var comparator = sortField.getComparator(10, Pruning.GREATER_THAN_OR_EQUAL_TO); + var leafComparator = comparator.getLeafComparator(reader.getContext().leaves().get(0)); + assertNotNull(leafComparator.competitiveIterator()); + reader.close(); w.close(); dir.close(); From ecea998a2c81fb8a37b2622ce5803d903bec92b2 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 31 Oct 2025 11:31:02 +0000 Subject: [PATCH 05/23] Add competitive sort tests to MapperTestCase --- .../LegacyGeoShapeFieldMapperTests.java | 5 ++ .../extras/MatchOnlyTextFieldMapperTests.java | 5 ++ .../extras/RankFeatureFieldMapperTests.java | 5 ++ .../extras/RankFeaturesFieldMapperTests.java | 5 ++ .../SearchAsYouTypeFieldMapperTests.java | 5 ++ .../extras/TokenCountFieldMapperTests.java | 5 ++ .../ICUCollationKeywordFieldMapperTests.java | 5 ++ .../AnnotatedTextFieldMapperTests.java | 5 ++ .../murmur3/Murmur3FieldMapperTests.java | 5 ++ .../index/mapper/BinaryFieldMapperTests.java | 5 ++ .../index/mapper/BooleanFieldMapperTests.java | 9 +++ .../mapper/CompletionFieldMapperTests.java | 5 ++ .../index/mapper/DateFieldMapperTests.java | 20 ++++++ .../mapper/GeoPointFieldMapperTests.java | 5 ++ .../index/mapper/IpFieldMapperTests.java | 9 +++ .../index/mapper/KeywordFieldMapperTests.java | 5 ++ .../index/mapper/RangeFieldMapperTests.java | 5 ++ .../index/mapper/TextFieldMapperTests.java | 4 ++ .../flattened/FlattenedFieldMapperTests.java | 5 ++ .../SyntheticVectorsMapperTestCase.java | 6 ++ .../index/mapper/MapperTestCase.java | 66 +++++++++++++++++++ .../index/mapper/NumberFieldMapperTests.java | 9 +++ .../mapper/HistogramFieldMapperTests.java | 5 ++ .../mapper/OffsetSourceFieldMapperTests.java | 5 ++ .../mapper/SemanticTextFieldMapperTests.java | 5 ++ .../PatternTextFieldMapperTests.java | 5 ++ ...AggregateMetricDoubleFieldMapperTests.java | 5 ++ .../ConstantKeywordFieldMapperTests.java | 5 ++ .../CountedKeywordFieldMapperTests.java | 5 ++ .../ExponentialHistogramFieldMapperTests.java | 5 ++ .../VersionStringFieldMapperTests.java | 5 ++ ...GeoShapeWithDocValuesFieldMapperTests.java | 5 ++ .../index/mapper/PointFieldMapperTests.java | 5 ++ .../index/mapper/ShapeFieldMapperTests.java | 5 ++ .../mapper/WildcardFieldMapperTests.java | 5 ++ 35 files changed, 263 insertions(+) diff --git a/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapperTests.java b/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapperTests.java index c97b0a28d22de..dcbf5bcb87e41 100644 --- a/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapperTests.java +++ b/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapperTests.java @@ -678,4 +678,9 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapperTests.java index ef72e234f8d6b..3a567490fc218 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapperTests.java @@ -385,4 +385,9 @@ public void testLoadSyntheticSourceFromStringOrBytesRef() throws IOException { } } } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/RankFeatureFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/RankFeatureFieldMapperTests.java index 917941b39e888..174bc44cbaecc 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/RankFeatureFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/RankFeatureFieldMapperTests.java @@ -215,4 +215,9 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/RankFeaturesFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/RankFeaturesFieldMapperTests.java index 6818b7936d124..c229752917560 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/RankFeaturesFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/RankFeaturesFieldMapperTests.java @@ -209,4 +209,9 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean syntheticSource) protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SearchAsYouTypeFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SearchAsYouTypeFieldMapperTests.java index 3e2bb422c2062..9965259e294da 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SearchAsYouTypeFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SearchAsYouTypeFieldMapperTests.java @@ -877,4 +877,9 @@ protected RandomIndexWriter indexWriterForSyntheticSource(Directory directory) t protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/TokenCountFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/TokenCountFieldMapperTests.java index 8105eee707cc3..b77ab875926e8 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/TokenCountFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/TokenCountFieldMapperTests.java @@ -264,4 +264,9 @@ public void testAggregationsDocValuesDisabled() throws IOException { })); assertAggregatableConsistency(mapperService.fieldType("field")); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/plugins/analysis-icu/src/test/java/org/elasticsearch/plugin/analysis/icu/ICUCollationKeywordFieldMapperTests.java b/plugins/analysis-icu/src/test/java/org/elasticsearch/plugin/analysis/icu/ICUCollationKeywordFieldMapperTests.java index 034a25e854298..275a70eb43f57 100644 --- a/plugins/analysis-icu/src/test/java/org/elasticsearch/plugin/analysis/icu/ICUCollationKeywordFieldMapperTests.java +++ b/plugins/analysis-icu/src/test/java/org/elasticsearch/plugin/analysis/icu/ICUCollationKeywordFieldMapperTests.java @@ -318,4 +318,9 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapperTests.java b/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapperTests.java index ae3d03c3a503f..b808cffd75c7a 100644 --- a/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapperTests.java +++ b/plugins/mapper-annotated-text/src/test/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapperTests.java @@ -704,4 +704,9 @@ protected void validateRoundTripReader(String syntheticSource, DirectoryReader r protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/plugins/mapper-murmur3/src/test/java/org/elasticsearch/index/mapper/murmur3/Murmur3FieldMapperTests.java b/plugins/mapper-murmur3/src/test/java/org/elasticsearch/index/mapper/murmur3/Murmur3FieldMapperTests.java index d04f2067d8aed..2978ee458dda9 100644 --- a/plugins/mapper-murmur3/src/test/java/org/elasticsearch/index/mapper/murmur3/Murmur3FieldMapperTests.java +++ b/plugins/mapper-murmur3/src/test/java/org/elasticsearch/index/mapper/murmur3/Murmur3FieldMapperTests.java @@ -148,4 +148,9 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BinaryFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BinaryFieldMapperTests.java index 7f257172638c2..8c3800bdf6132 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/BinaryFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/BinaryFieldMapperTests.java @@ -263,4 +263,9 @@ public int compareTo(BytesCompareUnsigned o) { protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java index 47161239a363d..76df9b6303109 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java @@ -409,4 +409,13 @@ public void execute() { } }; } + + @Override + protected List getSortShortcutSupport() { + return List.of( + // TODO: boolean field mapper uses a numeric comparator but is indexed with Terms + // so skipping doesn't work here. + new SortShortcutSupport(this::minimalMapping, this::writeField, false) + ); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java index 995127e2b3593..079521b26b666 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java @@ -985,4 +985,9 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java index 4d6368b712f98..f3e2633717a4b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java @@ -914,4 +914,24 @@ public void testSingletonLongBulkBlockReadingManyValues() throws Exception { } } } + + @Override + protected List getSortShortcutSupport() { + return List.of( + new SortShortcutSupport(b -> b.field("type", "date"), b -> b.field("field", "2025-10-30T00:00:00"), true), + new SortShortcutSupport(b -> b.field("type", "date_nanos"), b -> b.field("field", "2025-10-30T00:00:00"), true), + new SortShortcutSupport( + IndexVersion.fromId(5000099), + b -> b.field("type", "date"), + b -> b.field("field", "2025-10-30T00:00:00"), + false + ), + new SortShortcutSupport( + IndexVersion.fromId(5000099), + b -> b.field("type", "date_nanos"), + b -> b.field("field", "2025-10-30T00:00:00"), + false + ) + ); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java index 9ffe1ba394b0b..c560448f6bb33 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java @@ -719,4 +719,9 @@ public void testSyntheticSourceKeepArrays() { protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java index 2ca2a7f8eb62d..e0308b5fd8bbf 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java @@ -438,4 +438,13 @@ public void execute() { protected String randomSyntheticSourceKeep() { return "all"; } + + @Override + protected List getSortShortcutSupport() { + return List.of( + // TODO - shortcuts are disabled here, can we enable them? + new SortShortcutSupport(this::minimalMapping, this::writeField, false), + new SortShortcutSupport(IndexVersion.fromId(5000099), this::minimalMapping, this::writeField, false) + ); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index 520ec8a8c3400..641ddeb170f77 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -1219,4 +1219,9 @@ protected String randomSyntheticSourceKeep() { // Only option all keeps array source in ignored source. return randomFrom("all"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(new SortShortcutSupport(this::minimalMapping, this::writeField, true)); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java index 7555d1cd7fb48..6dd85a3f8c5e6 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java @@ -426,4 +426,9 @@ protected Object generateRandomInputValue(MappedFieldType ft) { assumeFalse("DocValuesFetcher doesn't work", true); return null; } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java index 970dd8886b93f..4947af5eeba11 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java @@ -1776,4 +1776,8 @@ public void testNormsEnabledWhenIndexModeIsTsdb_bwcCheck() throws IOException { assertThat(fieldType.omitNorms(), is(false)); } + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java index d9223a157b2ff..3939fcaa6e0e8 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java @@ -1016,4 +1016,9 @@ public void assertStoredFieldsEquals(String info, IndexReader leftReader, IndexR assertFalse(info, rightIterator.hasNext()); } } + + @Override + protected List getSortShortcutSupport() { + return List.of(new SortShortcutSupport(this::minimalMapping, this::writeField, true)); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/vectors/SyntheticVectorsMapperTestCase.java b/server/src/test/java/org/elasticsearch/index/mapper/vectors/SyntheticVectorsMapperTestCase.java index f7a23383f4e92..dbf4bd9846165 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/vectors/SyntheticVectorsMapperTestCase.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/vectors/SyntheticVectorsMapperTestCase.java @@ -25,6 +25,7 @@ import org.elasticsearch.xcontent.XContentType; import java.io.IOException; +import java.util.List; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; @@ -262,4 +263,9 @@ private void assertSyntheticVectors(String mapping, BytesReference source, XCont } } } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index 3ef66cebc1f14..f7ee3dcdbda43 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -22,6 +22,7 @@ import org.apache.lucene.index.NoMergePolicy; import org.apache.lucene.search.FieldExistsQuery; import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.Pruning; import org.apache.lucene.search.Query; import org.apache.lucene.search.Sort; import org.apache.lucene.search.SortField; @@ -64,6 +65,7 @@ import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.field.DocValuesScriptFieldFactory; import org.elasticsearch.search.DocValueFormat; +import org.elasticsearch.search.MultiValueMode; import org.elasticsearch.search.lookup.LeafStoredFieldsLookup; import org.elasticsearch.search.lookup.SearchLookup; import org.elasticsearch.search.lookup.Source; @@ -1697,4 +1699,68 @@ protected T compileOtherScript(Script script, ScriptContext context) { */ abstract ScriptFactory nonEmptyFieldScript(); } + + /** + * Return a list of scenarios where sorts on this field should or should not produce + * competitive iterators. Field types that do not support sorting should return an + * empty List. + */ + protected abstract List getSortShortcutSupport(); + + public record SortShortcutSupport( + IndexVersion indexVersion, + Settings settings, + CheckedConsumer mappings, + CheckedConsumer document, + boolean supportsShortcut + ) { + public SortShortcutSupport( + CheckedConsumer mappings, + CheckedConsumer document, + boolean supportsShortcut + ) { + this(IndexVersion.current(), SETTINGS, mappings, document, supportsShortcut); + } + + public SortShortcutSupport( + IndexVersion indexVersion, + CheckedConsumer mappings, + CheckedConsumer document, + boolean supportsShortcut + ) { + this(indexVersion, SETTINGS, mappings, document, supportsShortcut); + } + } + + public final void testSortShortcuts() throws IOException { + List tests = getSortShortcutSupport(); + assumeTrue("Sort shortcuts not supported", tests != null && tests.isEmpty() == false); + + for (SortShortcutSupport sortShortcutSupport : tests) { + MapperService mapperService = createMapperService(sortShortcutSupport.indexVersion(), sortShortcutSupport.settings, () -> true); + merge(mapperService, fieldMapping(sortShortcutSupport.mappings)); + withLuceneIndex(mapperService, iw -> { + iw.addDocument( + mapperService.documentParser() + .parseDocument(source(sortShortcutSupport.document()), mapperService.mappingLookup()) + .rootDoc() + ); + }, reader -> { + IndexSearcher searcher = newSearcher(reader); + MappedFieldType ft = mapperService.fieldType("field"); + SortField sortField = ft.fielddataBuilder(new FieldDataContext("", mapperService.getIndexSettings(), () -> { + throw new UnsupportedOperationException(); + }, Set::of, MappedFieldType.FielddataOperation.SEARCH)) + .build(null, null) + .sortField(getVersion(), null, MultiValueMode.MIN, null, false); + var comparator = sortField.getComparator(10, Pruning.GREATER_THAN_OR_EQUAL_TO); + var leafComparator = comparator.getLeafComparator(searcher.getLeafContexts().getFirst()); + if (sortShortcutSupport.supportsShortcut) { + assertNotNull(leafComparator.competitiveIterator()); + } else { + assertNull(leafComparator.competitiveIterator()); + } + }); + } + } } diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java index 18e60c008db96..2f16534908b2e 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -16,6 +16,7 @@ import org.elasticsearch.core.Tuple; import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.IndexVersion; import org.elasticsearch.script.DoubleFieldScript; import org.elasticsearch.script.LongFieldScript; import org.elasticsearch.script.Script; @@ -496,4 +497,12 @@ public List invalidExample() throws IOException { return List.of(); } } + + @Override + protected List getSortShortcutSupport() { + return List.of( + new SortShortcutSupport(this::minimalMapping, this::writeField, true), + new SortShortcutSupport(IndexVersion.fromId(5000099), this::minimalMapping, this::writeField, false) + ); + } } diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java index ef096f344f352..b26b33cc01544 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java @@ -523,4 +523,9 @@ public List invalidExample() throws IOException { public void testSyntheticSourceKeepArrays() { // The mapper expects to parse an array of values by default, it's not compatible with array of arrays. } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/OffsetSourceFieldMapperTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/OffsetSourceFieldMapperTests.java index b3ee52c8dede5..384a321d524be 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/OffsetSourceFieldMapperTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/OffsetSourceFieldMapperTests.java @@ -214,4 +214,9 @@ public void testInvalidOffsets() throws IOException { }))); assertThat(exc.getCause().getCause().getCause().getMessage(), containsString("Illegal offsets")); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java index 273a202baa7d6..2c9c404fcb275 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java @@ -2369,4 +2369,9 @@ private static void assertSparseFeatures(LuceneDocument doc, String fieldName, i private void givenModelSettings(String inferenceId, MinimalServiceSettings modelSettings) { when(globalModelRegistry.getMinimalServiceSettings(inferenceId)).thenReturn(modelSettings); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/patterntext/PatternTextFieldMapperTests.java b/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/patterntext/PatternTextFieldMapperTests.java index 1b3ccb376febf..8a684122cb658 100644 --- a/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/patterntext/PatternTextFieldMapperTests.java +++ b/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/patterntext/PatternTextFieldMapperTests.java @@ -470,4 +470,9 @@ public void testSyntheticSourceKeepArrays() { protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapperTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapperTests.java index 874ced5436f69..b1e31882fbf1a 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapperTests.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapperTests.java @@ -618,4 +618,9 @@ public void testSyntheticSourceKeepArrays() { protected boolean supportsCopyTo() { return false; } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/mapper-constant-keyword/src/test/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapperTests.java b/x-pack/plugin/mapper-constant-keyword/src/test/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapperTests.java index d5405018dd64c..cdbfc6c3b9582 100644 --- a/x-pack/plugin/mapper-constant-keyword/src/test/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapperTests.java +++ b/x-pack/plugin/mapper-constant-keyword/src/test/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapperTests.java @@ -346,4 +346,9 @@ protected boolean supportsEmptyInputArray() { protected boolean addsValueWhenNotSupplied() { return true; } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/mapper-counted-keyword/src/test/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapperTests.java b/x-pack/plugin/mapper-counted-keyword/src/test/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapperTests.java index d6b762ab4b79e..5c26222ff1618 100644 --- a/x-pack/plugin/mapper-counted-keyword/src/test/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapperTests.java +++ b/x-pack/plugin/mapper-counted-keyword/src/test/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapperTests.java @@ -219,4 +219,9 @@ public void testDisableIndex() throws IOException { assertEquals(IndexOptions.NONE, fields.get(0).fieldType().indexOptions()); assertEquals(DocValuesType.SORTED_SET, fields.get(0).fieldType().docValuesType()); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java b/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java index 730d0b591ec7a..f782e7f30b7e3 100644 --- a/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java +++ b/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java @@ -605,4 +605,9 @@ public void testSyntheticSourceKeepArrays() { protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not yet implemented"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/mapper-version/src/test/java/org/elasticsearch/xpack/versionfield/VersionStringFieldMapperTests.java b/x-pack/plugin/mapper-version/src/test/java/org/elasticsearch/xpack/versionfield/VersionStringFieldMapperTests.java index 710b6652ed05d..61d6021005f56 100644 --- a/x-pack/plugin/mapper-version/src/test/java/org/elasticsearch/xpack/versionfield/VersionStringFieldMapperTests.java +++ b/x-pack/plugin/mapper-version/src/test/java/org/elasticsearch/xpack/versionfield/VersionStringFieldMapperTests.java @@ -198,4 +198,9 @@ public List invalidExample() throws IOException { return List.of(); } } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java index 9482f45063b64..d656f407b3243 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java @@ -553,4 +553,9 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java index 70df2763e30c6..3cb9f531b23dc 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java @@ -553,4 +553,9 @@ protected Object[] getThreeEncodedSampleValues() { protected boolean supportsBulkLongBlockReading() { return true; } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java index ee6f6bc8b70c0..9530f24a153a1 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java @@ -372,4 +372,9 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } diff --git a/x-pack/plugin/wildcard/src/test/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapperTests.java b/x-pack/plugin/wildcard/src/test/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapperTests.java index be216c55d49cf..0690d38fe79ce 100644 --- a/x-pack/plugin/wildcard/src/test/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapperTests.java +++ b/x-pack/plugin/wildcard/src/test/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapperTests.java @@ -1272,4 +1272,9 @@ public List invalidExample() throws IOException { return List.of(); } } + + @Override + protected List getSortShortcutSupport() { + return List.of(); + } } From 842d14cb6085b9ffcf39cab1745c0f16d12dbdcf Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 31 Oct 2025 13:56:38 +0000 Subject: [PATCH 06/23] iter --- .../index/mapper/extras/ScaledFloatFieldMapperTests.java | 8 ++++++++ .../xpack/unsignedlong/UnsignedLongFieldMapperTests.java | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapperTests.java index dce105f339a52..7f988ebcb04f4 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapperTests.java @@ -649,4 +649,12 @@ private double encodeDecode(double value, double scalingFactor) { private static double randomValue() { return randomBoolean() ? randomDoubleBetween(-Double.MAX_VALUE, Double.MAX_VALUE, true) : randomFloat(); } + + @Override + protected List getSortShortcutSupport() { + return List.of( + // TODO doubles currently disable pruning, can we re-enable? + new SortShortcutSupport(this::minimalMapping, this::writeField, false) + ); + } } diff --git a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java index cb28a6168c561..f8aed69688e81 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java +++ b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java @@ -520,4 +520,11 @@ protected Object[] getThreeEncodedSampleValues() { protected boolean supportsBulkLongBlockReading() { return true; } + + @Override + protected List getSortShortcutSupport() { + return List.of( + new SortShortcutSupport(this::minimalMapping, this::writeField, true) + ); + } } From b04481e076d9df00b2dcaea74642da81e0d3e1a6 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 31 Oct 2025 15:49:29 +0000 Subject: [PATCH 07/23] Add competitive iterator check for logsdb-style timestamp field --- .../index/mapper/DateFieldMapperTests.java | 9 +++++++++ .../index/mapper/MapperServiceTestCase.java | 10 ++++++++++ .../index/mapper/MapperTestCase.java | 17 ++++++++++++----- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java index f3e2633717a4b..5a117765424ab 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java @@ -918,6 +918,15 @@ public void testSingletonLongBulkBlockReadingManyValues() throws Exception { @Override protected List getSortShortcutSupport() { return List.of( + new SortShortcutSupport( + IndexVersion.current(), + Settings.builder().put("index.mode", "logsdb").put("index.logsdb.sort_on_host_name", true).build(), + "@timestamp", + b -> b.field("type", "date").field("ignore_malformed", false), + b -> b.startObject("host.name").field("type", "keyword").endObject(), + b -> b.field("@timestamp", "2025-10-30T00:00:00").field("host.name", "foo"), + true + ), new SortShortcutSupport(b -> b.field("type", "date"), b -> b.field("field", "2025-10-30T00:00:00"), true), new SortShortcutSupport(b -> b.field("type", "date_nanos"), b -> b.field("field", "2025-10-30T00:00:00"), true), new SortShortcutSupport( diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java index 8ccab79c1c6ae..02afc6981a198 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java @@ -17,6 +17,7 @@ import org.apache.lucene.index.LeafReader; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Query; +import org.apache.lucene.search.Sort; import org.apache.lucene.store.Directory; import org.apache.lucene.tests.index.RandomIndexWriter; import org.elasticsearch.TransportVersion; @@ -40,6 +41,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.IndexSortConfig; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.analysis.AnalyzerScope; import org.elasticsearch.index.analysis.IndexAnalyzers; @@ -395,9 +397,17 @@ protected static void withLuceneIndex( CheckedConsumer builder, CheckedConsumer test ) throws IOException { + IndexSortConfig sortConfig = new IndexSortConfig(mapperService.getIndexSettings()); + Sort indexSort = sortConfig.buildIndexSort( + mapperService::fieldType, + (ft, s) -> ft.fielddataBuilder(FieldDataContext.noRuntimeFields("")).build(null, null) + ); IndexWriterConfig iwc = new IndexWriterConfig(IndexShard.buildIndexAnalyzer(mapperService)).setCodec( new PerFieldMapperCodec(Zstd814StoredFieldsFormat.Mode.BEST_SPEED, mapperService, BigArrays.NON_RECYCLING_INSTANCE) ); + if (indexSort != null) { + iwc.setIndexSort(indexSort); + } try (Directory dir = newDirectory(); RandomIndexWriter iw = new RandomIndexWriter(random(), dir, iwc)) { builder.accept(iw); try (DirectoryReader reader = iw.getReader()) { diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index f7ee3dcdbda43..a0f4b7c4b4e06 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -1710,7 +1710,9 @@ protected T compileOtherScript(Script script, ScriptContext context) { public record SortShortcutSupport( IndexVersion indexVersion, Settings settings, - CheckedConsumer mappings, + String fieldname, + CheckedConsumer fieldMapping, + CheckedConsumer additionalMappings, CheckedConsumer document, boolean supportsShortcut ) { @@ -1719,7 +1721,7 @@ public SortShortcutSupport( CheckedConsumer document, boolean supportsShortcut ) { - this(IndexVersion.current(), SETTINGS, mappings, document, supportsShortcut); + this(IndexVersion.current(), SETTINGS, "field", mappings, b -> {}, document, supportsShortcut); } public SortShortcutSupport( @@ -1728,7 +1730,7 @@ public SortShortcutSupport( CheckedConsumer document, boolean supportsShortcut ) { - this(indexVersion, SETTINGS, mappings, document, supportsShortcut); + this(indexVersion, SETTINGS, "field", mappings, b -> {}, document, supportsShortcut); } } @@ -1738,7 +1740,12 @@ public final void testSortShortcuts() throws IOException { for (SortShortcutSupport sortShortcutSupport : tests) { MapperService mapperService = createMapperService(sortShortcutSupport.indexVersion(), sortShortcutSupport.settings, () -> true); - merge(mapperService, fieldMapping(sortShortcutSupport.mappings)); + merge(mapperService, mapping(b -> { + b.startObject(sortShortcutSupport.fieldname); + sortShortcutSupport.fieldMapping.accept(b); + b.endObject(); + sortShortcutSupport.additionalMappings.accept(b); + })); withLuceneIndex(mapperService, iw -> { iw.addDocument( mapperService.documentParser() @@ -1747,7 +1754,7 @@ public final void testSortShortcuts() throws IOException { ); }, reader -> { IndexSearcher searcher = newSearcher(reader); - MappedFieldType ft = mapperService.fieldType("field"); + MappedFieldType ft = mapperService.fieldType(sortShortcutSupport.fieldname); SortField sortField = ft.fielddataBuilder(new FieldDataContext("", mapperService.getIndexSettings(), () -> { throw new UnsupportedOperationException(); }, Set::of, MappedFieldType.FielddataOperation.SEARCH)) From 2fd86b587140a3e4b85b62af8e77ed1841345846 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 3 Nov 2025 11:53:52 +0100 Subject: [PATCH 08/23] NumericComparator: immediately check whether a segment is comparative with the recorded bottom. When construction a new CompetitiveDISIBuilder, then check whether global min/max points or global min/max doc values skipper are comparative with the bottom. If so, then update competitiveIterator with an empty iterator, because no documents will have a value that is competitive with the current recorded bottom in the current segment. Doing this at CompetitiveDISIBuilder construction is cheap and allows to immediately prune, instead of waiting until doUpdateCompetitiveIterator(...) is invoked. --- .../comparators/XNumericComparator.java | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java index 84cab89743351..29eaa221c8002 100644 --- a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java @@ -317,8 +317,7 @@ private class PointsCompetitiveDISIBuilder extends CompetitiveDISIBuilder { // helps to be conservative about increasing the sampling interval private int tryUpdateFailCount = 0; - @SuppressWarnings("checkstyle:RedundantModifier") - public PointsCompetitiveDISIBuilder(PointValues pointValues, NumericLeafComparator comparator) { + PointsCompetitiveDISIBuilder(PointValues pointValues, NumericLeafComparator comparator) throws IOException { super(comparator); LeafReaderContext context = comparator.context; FieldInfo info = context.reader().getFieldInfos().fieldInfo(field); @@ -342,6 +341,7 @@ public PointsCompetitiveDISIBuilder(PointValues pointValues, NumericLeafComparat + bytesCount); } this.pointValues = pointValues; + postInitializeCompetitiveIterator(); } @Override @@ -362,6 +362,25 @@ protected int docCount() { return pointValues.getDocCount(); } + /** + * If queue is full and global min/max point values are not competitive with bottom then set an + * empty iterator as competitive iterator. + * + * @throws IOException i/o exception while fetching min and max values from point values + */ + void postInitializeCompetitiveIterator() throws IOException { + if (queueFull) { + long bottom = leafComparator.bottomAsComparableLong(); + long minValue = sortableBytesToLong(pointValues.getMinPackedValue()); + long maxValue = sortableBytesToLong(pointValues.getMaxPackedValue()); + if (reverse == false && bottom < minValue) { + competitiveIterator.update(DocIdSetIterator.empty()); + } else if (reverse && bottom > maxValue) { + competitiveIterator.update(DocIdSetIterator.empty()); + } + } + } + @Override protected void doUpdateCompetitiveIterator() throws IOException { DocIdSetBuilder result = new DocIdSetBuilder(maxDoc); @@ -476,8 +495,7 @@ private class DVSkipperCompetitiveDISIBuilder extends CompetitiveDISIBuilder { private final DocValuesSkipper skipper; private final TwoPhaseIterator innerTwoPhase; - @SuppressWarnings("checkstyle:RedundantModifier") - public DVSkipperCompetitiveDISIBuilder( + DVSkipperCompetitiveDISIBuilder( DocValuesSkipper skipper, NumericLeafComparator leafComparator) throws IOException { super(leafComparator); this.skipper = skipper; @@ -496,6 +514,7 @@ public float matchCost() { return 2; // 2 comparisons } }; + postInitializeCompetitiveIterator(); } @Override @@ -503,6 +522,21 @@ protected int docCount() { return skipper.docCount(); } + /** + * If queue is full and global min/max skipper are not competitive with bottom then set an empty + * iterator as competitive iterator. + */ + void postInitializeCompetitiveIterator() { + if (queueFull) { + long bottom = leafComparator.bottomAsComparableLong(); + if (reverse == false && bottom < skipper.minValue()) { + competitiveIterator.update(DocIdSetIterator.empty()); + } else if (reverse && bottom > skipper.maxValue()) { + competitiveIterator.update(DocIdSetIterator.empty()); + } + } + } + @Override protected void doUpdateCompetitiveIterator() { TwoPhaseIterator twoPhaseIterator = From f9f81910b773ca505bc2255aa6fdf23596169fc0 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 3 Nov 2025 15:39:40 +0000 Subject: [PATCH 09/23] Add indexSort method to IndexFieldData --- .../elasticsearch/index/IndexSortConfig.java | 2 +- .../index/fielddata/IndexFieldData.java | 12 +++++ .../fielddata/IndexNumericFieldData.java | 54 ++++++++++++++++++- .../LongValuesComparatorSource.java | 6 ++- .../SecondarySortIterator.java | 8 +-- .../comparators/XNumericComparator.java | 3 +- .../search/sort/FieldSortBuilder.java | 2 +- 7 files changed, 76 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/IndexSortConfig.java b/server/src/main/java/org/elasticsearch/index/IndexSortConfig.java index fd445d470837c..37742a4f5104d 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexSortConfig.java +++ b/server/src/main/java/org/elasticsearch/index/IndexSortConfig.java @@ -375,7 +375,7 @@ public Sort buildIndexSort( if (fieldData == null) { throw new IllegalArgumentException("docvalues not found for index sort field:[" + sortSpec.field + "]"); } - sortFields[i] = fieldData.sortField(this.indexCreatedVersion, sortSpec.missingValue, mode, null, reverse); + sortFields[i] = fieldData.indexSortField(this.indexCreatedVersion, sortSpec.missingValue, mode, reverse); validateIndexSortField(sortFields[i]); } return new Sort(sortFields); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldData.java index 5f6cc93063d87..13bffa6dff0f1 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldData.java @@ -83,6 +83,18 @@ default SortField sortField( */ SortField sortField(@Nullable Object missingValue, MultiValueMode sortMode, Nested nested, boolean reverse); + /** + * Returns the {@link SortField} to use for IndexSorting, if supported + */ + default SortField indexSortField( + IndexVersion indexCreatedVersion, + @Nullable Object missingValue, + MultiValueMode sortMode, + boolean reverse + ) { + return sortField(indexCreatedVersion, missingValue, sortMode, null, reverse); + } + /** * Build a sort implementation specialized for aggregations. */ diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java index f6e03c40db94f..77999b221f940 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java @@ -88,6 +88,7 @@ public final ValuesSourceType getValuesSourceType() { * match the field's numericType. */ public final SortField sortField( + boolean isIndexSort, NumericType targetNumericType, Object missingValue, MultiValueMode sortMode, @@ -108,6 +109,9 @@ public final SortField sortField( || targetNumericType != getNumericType(); boolean canUseOptimizedSort = canUseOptimizedSort(indexType()); if (sortRequiresCustomComparator() || requiresCustomComparator) { + if (isIndexSort) { + return new SortField(getFieldName(), source, reverse); + } return new SortField(getFieldName(), source, reverse) { @Override public FieldComparator getComparator(int numHits, Pruning pruning) { @@ -119,6 +123,11 @@ public FieldComparator getComparator(int numHits, Pruning pruning) { SortedNumericSelector.Type selectorType = sortMode == MultiValueMode.MAX ? SortedNumericSelector.Type.MAX : SortedNumericSelector.Type.MIN; + if (isIndexSort) { + SortField sortField = new SortedNumericSortField(getFieldName(), getNumericType().sortFieldType, reverse, selectorType); + sortField.setMissingValue(source.missingObject(missingValue, reverse)); + return sortField; + } SortField sortField = new SortedNumericSortField(getFieldName(), getNumericType().sortFieldType, reverse, selectorType) { @Override public FieldComparator getComparator(int numHits, Pruning pruning) { @@ -148,7 +157,48 @@ private static boolean canUseOptimizedSort(IndexType indexType) { @Override public final SortField sortField(Object missingValue, MultiValueMode sortMode, Nested nested, boolean reverse) { - return sortField(getNumericType(), missingValue, sortMode, nested, reverse); + return sortField(false, getNumericType(), missingValue, sortMode, nested, reverse); + } + + @Override + public SortField indexSortField(IndexVersion indexCreatedVersion, Object missingValue, MultiValueMode sortMode, boolean reverse) { + SortField sortField = sortField(true, getNumericType(), missingValue, sortMode, null, reverse); + if (getNumericType() == NumericType.DATE_NANOSECONDS + && indexCreatedVersion.before(IndexVersions.V_7_14_0) + && missingValue.equals("_last") + && Long.valueOf(0L).equals(sortField.getMissingValue())) { + // 7.14 changed the default missing value of sort on date_nanos, from Long.MIN_VALUE + // to 0L - for compatibility we require to a missing value of MIN_VALUE to allow to + // open the index. + sortField.setMissingValue(Long.MIN_VALUE); + return sortField; + } else if (getNumericType().sortFieldType != SortField.Type.INT + // we introduced INT sort type in 8.19 and from 9.1 + || indexCreatedVersion.onOrAfter(IndexVersions.INDEX_INT_SORT_INT_TYPE) + || indexCreatedVersion.between(IndexVersions.INDEX_INT_SORT_INT_TYPE_8_19, UPGRADE_TO_LUCENE_10_0_0)) { + return sortField; + } + if ((sortField instanceof SortedNumericSortField) == false) { + return sortField; + } + // if the index was created before 8.19, or in 9.0 + // we need to rewrite the sort field to use LONG sort type + + // Rewrite INT sort to LONG sort. + // Before indices used TYPE.LONG for index sorting on integer field, + // and this is stored in their index writer config on disk and can't be modified. + // Now sortField() returns TYPE.INT when sorting on integer field, + // but to support sorting on old indices, we need to rewrite this sort to TYPE.LONG. + XFieldComparatorSource longSource = comparatorSource(NumericType.LONG, missingValue, sortMode, null); + SortedNumericSortField numericSortField = (SortedNumericSortField) sortField; + SortedNumericSortField rewrittenSortField = new SortedNumericSortField( + sortField.getField(), + SortField.Type.LONG, + sortField.getReverse(), + numericSortField.getSelector() + ); + rewrittenSortField.setMissingValue(longSource.missingObject(missingValue, reverse)); + return rewrittenSortField; } @Override @@ -186,6 +236,7 @@ public SortField sortField( // and this is stored in their index writer config on disk and can't be modified. // Now sortField() returns TYPE.INT when sorting on integer field, // but to support sorting on old indices, we need to rewrite this sort to TYPE.LONG. + XFieldComparatorSource longSource = comparatorSource(NumericType.LONG, missingValue, sortMode, nested); SortedNumericSortField numericSortField = (SortedNumericSortField) sortField; SortedNumericSortField rewrittenSortField = new SortedNumericSortField( sortField.getField(), @@ -199,7 +250,6 @@ public FieldComparator getComparator(int numHits, Pruning pruning) { return super.getComparator(numHits, Pruning.NONE); } }; - XFieldComparatorSource longSource = comparatorSource(NumericType.LONG, missingValue, sortMode, nested); rewrittenSortField.setMissingValue(longSource.missingObject(missingValue, reverse)); return rewrittenSortField; } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index 465a69503abec..1db4ef1feb078 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java @@ -224,7 +224,11 @@ public int nextDoc() throws IOException { @Override public int advance(int target) throws IOException { - throw new UnsupportedOperationException(); + // All documents are guaranteed to have a value, as all invocations of getLongValues + // always return `true` from `advanceExact()` + boolean hasValue = longValues.advanceExact(target); + assert hasValue : "LongValuesComparatorSource#wrap called with a LongValues that has missing values"; + return target; } @Override diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java index c33a7bef3515e..818126239f83e 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java @@ -46,11 +46,11 @@ class SecondarySortIterator extends DocIdSetIterator { this.valueSkipper = valueSkipper; this.primaryFieldSkipper = primaryFieldSkipper; this.minValue = minValue; - this.maxValue = maxValue; + this.maxValue = maxValue; - valueFieldUpTo = valueSkipper.maxDocID(0); - primaryFieldUpTo = primaryFieldSkipper.maxDocID(0); - } + valueFieldUpTo = valueSkipper.maxDocID(0); + primaryFieldUpTo = primaryFieldSkipper.maxDocID(0); + } @Override public int docID() { diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java index 84cab89743351..bc8e98beb2226 100644 --- a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java @@ -317,8 +317,7 @@ private class PointsCompetitiveDISIBuilder extends CompetitiveDISIBuilder { // helps to be conservative about increasing the sampling interval private int tryUpdateFailCount = 0; - @SuppressWarnings("checkstyle:RedundantModifier") - public PointsCompetitiveDISIBuilder(PointValues pointValues, NumericLeafComparator comparator) { + PointsCompetitiveDISIBuilder(PointValues pointValues, NumericLeafComparator comparator) { super(comparator); LeafReaderContext context = comparator.context; FieldInfo info = context.reader().getFieldInfos().fieldInfo(field); diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index 8758aa6dfbe1a..bd12161330ab2 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -355,7 +355,7 @@ public SortFieldAndFormat build(SearchExecutionContext context) throws IOExcepti } IndexNumericFieldData numericFieldData = (IndexNumericFieldData) fieldData; NumericType resolvedType = resolveNumericType(numericType); - field = numericFieldData.sortField(resolvedType, missing, localSortMode(), nested, reverse); + field = numericFieldData.sortField(false, resolvedType, missing, localSortMode(), nested, reverse); isNanosecond = resolvedType == NumericType.DATE_NANOSECONDS; } else { field = fieldData.sortField(context.indexVersionCreated(), missing, localSortMode(), nested, reverse); From 8603d58dfcdfdd0b05c3a381e5450c1d51fd59b8 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 3 Nov 2025 16:34:49 +0000 Subject: [PATCH 10/23] Update docs/changelog/137533.yaml --- docs/changelog/137533.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/137533.yaml diff --git a/docs/changelog/137533.yaml b/docs/changelog/137533.yaml new file mode 100644 index 0000000000000..cfd603ee4950f --- /dev/null +++ b/docs/changelog/137533.yaml @@ -0,0 +1,5 @@ +pr: 137533 +summary: Speed up sorts on secondary sort fields +area: Search +type: enhancement +issues: [] From 0595fffd816382e8c4a8a5b6bdbe42176dc2ae12 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Mon, 3 Nov 2025 16:41:58 +0000 Subject: [PATCH 11/23] [CI] Auto commit changes from spotless --- .../fielddata/IndexNumericFieldData.java | 4 +- .../SecondarySortIterator.java | 2 +- .../lucene/comparators/XLongComparator.java | 6 +- .../comparators/XNumericComparator.java | 176 ++++++++---------- .../XUpdateableDocIdSetIterator.java | 1 - .../xpack/logsdb/LogsdbSortConfigIT.java | 4 +- .../UnsignedLongFieldMapperTests.java | 4 +- 7 files changed, 86 insertions(+), 111 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java index 77999b221f940..503cd1a29775e 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java @@ -176,8 +176,8 @@ public SortField indexSortField(IndexVersion indexCreatedVersion, Object missing // we introduced INT sort type in 8.19 and from 9.1 || indexCreatedVersion.onOrAfter(IndexVersions.INDEX_INT_SORT_INT_TYPE) || indexCreatedVersion.between(IndexVersions.INDEX_INT_SORT_INT_TYPE_8_19, UPGRADE_TO_LUCENE_10_0_0)) { - return sortField; - } + return sortField; + } if ((sortField instanceof SortedNumericSortField) == false) { return sortField; } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java index 818126239f83e..405cc58080751 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java @@ -90,7 +90,7 @@ public int advance(int target) throws IOException { int upTo = valueFieldUpTo; if (maxValue < this.minValue) { // We've moved past the end of the valid values in the secondary sort field - // for this primary value. Advance the primary skipper to find the starting point + // for this primary value. Advance the primary skipper to find the starting point // for the next primary value, where the secondary field values will have reset primaryFieldSkipper.advance(target); primaryFieldUpTo = primaryFieldSkipper.maxDocID(0); diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java index 6caa94382a6bd..24bdf921efb16 100644 --- a/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java @@ -23,8 +23,7 @@ public class XLongComparator extends XNumericComparator { protected long topValue; protected long bottom; - public XLongComparator( - int numHits, String field, Long missingValue, boolean reverse, Pruning pruning) { + public XLongComparator(int numHits, String field, Long missingValue, boolean reverse, Pruning pruning) { super(field, missingValue != null ? missingValue : 0L, reverse, pruning, Long.BYTES); values = new long[numHits]; } @@ -68,7 +67,8 @@ public LongLeafComparator(LeafReaderContext context) throws IOException { } @Override - protected XNumericComparator.CompetitiveDISIBuilder buildCompetitiveDISIBuilder(LeafReaderContext context) throws IOException { + protected XNumericComparator.CompetitiveDISIBuilder buildCompetitiveDISIBuilder(LeafReaderContext context) + throws IOException { return super.buildCompetitiveDISIBuilder(context); } diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java index 29eaa221c8002..040f6a769752b 100644 --- a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java @@ -55,8 +55,7 @@ public abstract class XNumericComparator extends FieldComparat protected boolean queueFull; protected Pruning pruning; - protected XNumericComparator( - String field, T missingValue, boolean reverse, Pruning pruning, int bytesCount) { + protected XNumericComparator(String field, T missingValue, boolean reverse, Pruning pruning, int bytesCount) { this.field = field; this.missingValue = missingValue; this.missingValueAsLong = missingValueAsComparableLong(); @@ -129,8 +128,7 @@ protected CompetitiveDISIBuilder buildCompetitiveDISIBuilder(LeafReaderContext c * @return numeric doc values for the field in this segment. * @throws IOException If there is a low-level I/O error */ - protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) - throws IOException { + protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) throws IOException { return DocValues.getNumeric(context.reader(), field); } @@ -217,8 +215,7 @@ final void updateCompetitiveIterator() throws IOException { updateCounter++; // Start sampling if we get called too much - if (updateCounter > 256 - && (updateCounter & (currentSkipInterval - 1)) != currentSkipInterval - 1) { + if (updateCounter > 256 && (updateCounter & (currentSkipInterval - 1)) != currentSkipInterval - 1) { return; } @@ -262,18 +259,12 @@ private void encodeBottom() { private void encodeTop() { if (reverse == false) { minValueAsLong = leafComparator.topAsComparableLong(); - if (singleSort - && pruning == Pruning.GREATER_THAN_OR_EQUAL_TO - && queueFull - && minValueAsLong != Long.MAX_VALUE) { + if (singleSort && pruning == Pruning.GREATER_THAN_OR_EQUAL_TO && queueFull && minValueAsLong != Long.MAX_VALUE) { minValueAsLong++; } } else { maxValueAsLong = leafComparator.topAsComparableLong(); - if (singleSort - && pruning == Pruning.GREATER_THAN_OR_EQUAL_TO - && queueFull - && maxValueAsLong != Long.MIN_VALUE) { + if (singleSort && pruning == Pruning.GREATER_THAN_OR_EQUAL_TO && queueFull && maxValueAsLong != Long.MIN_VALUE) { maxValueAsLong--; } } @@ -286,10 +277,9 @@ boolean isMissingValueCompetitive() { int result = Long.compare(missingValueAsLong, leafComparator.bottomAsComparableLong()); // in reverse (desc) sort missingValue is competitive when it's greater or equal to bottom, // in asc sort missingValue is competitive when it's smaller or equal to bottom - final boolean competitive = - reverse - ? (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO ? result > 0 : result >= 0) - : (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO ? result < 0 : result <= 0); + final boolean competitive = reverse + ? (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO ? result > 0 : result >= 0) + : (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO ? result < 0 : result <= 0); if (competitive == false) { return false; } @@ -323,12 +313,10 @@ private class PointsCompetitiveDISIBuilder extends CompetitiveDISIBuilder { 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"); + "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"); + throw new IllegalArgumentException("Field " + field + " is indexed with multiple dimensions, sorting is not supported"); } else if (info.getPointNumBytes() != bytesCount) { throw new IllegalArgumentException( "Field " @@ -338,7 +326,8 @@ private class PointsCompetitiveDISIBuilder extends CompetitiveDISIBuilder { + " bytes per dimension, but " + this + " expected " - + bytesCount); + + bytesCount + ); } this.pointValues = pointValues; postInitializeCompetitiveIterator(); @@ -348,8 +337,7 @@ private class PointsCompetitiveDISIBuilder extends CompetitiveDISIBuilder { void setScorer(Scorable scorer) throws IOException { if (iteratorCost == -1) { if (scorer instanceof Scorer) { - iteratorCost = - ((Scorer) scorer).iterator().cost(); // starting iterator cost is the scorer's cost + iteratorCost = ((Scorer) scorer).iterator().cost(); // starting iterator cost is the scorer's cost } else { iteratorCost = maxDoc; } @@ -384,78 +372,75 @@ void postInitializeCompetitiveIterator() throws IOException { @Override protected void doUpdateCompetitiveIterator() throws IOException { DocIdSetBuilder result = new DocIdSetBuilder(maxDoc); - PointValues.IntersectVisitor visitor = - new PointValues.IntersectVisitor() { - DocIdSetBuilder.BulkAdder adder; + PointValues.IntersectVisitor visitor = new PointValues.IntersectVisitor() { + DocIdSetBuilder.BulkAdder adder; - @Override - public void grow(int count) { - adder = result.grow(count); - } + @Override + public void grow(int count) { + adder = result.grow(count); + } - @Override - public void visit(int docID) { - if (docID <= maxDocVisited) { - return; // Already visited or skipped - } - adder.add(docID); + @Override + public void visit(int docID) { + if (docID <= maxDocVisited) { + return; // Already visited or skipped } + adder.add(docID); + } - @Override - public void visit(int docID, byte[] packedValue) { - if (docID <= maxDocVisited) { - return; // already visited or skipped - } - long l = sortableBytesToLong(packedValue); - if (l >= minValueAsLong && l <= maxValueAsLong) { - adder.add(docID); // doc is competitive - } + @Override + public void visit(int docID, byte[] packedValue) { + if (docID <= maxDocVisited) { + return; // already visited or skipped + } + long l = sortableBytesToLong(packedValue); + if (l >= minValueAsLong && l <= maxValueAsLong) { + adder.add(docID); // doc is competitive } + } - @Override - public void visit(DocIdSetIterator iterator) throws IOException { - if (iterator.advance(maxDocVisited + 1) != DocIdSetIterator.NO_MORE_DOCS) { - adder.add(iterator.docID()); - adder.add(iterator); - } + @Override + public void visit(DocIdSetIterator iterator) throws IOException { + if (iterator.advance(maxDocVisited + 1) != DocIdSetIterator.NO_MORE_DOCS) { + adder.add(iterator.docID()); + adder.add(iterator); } + } - @Override - public void visit(IntsRef ref) { - adder.add(ref, maxDocVisited + 1); + @Override + public void visit(IntsRef ref) { + adder.add(ref, maxDocVisited + 1); + } + + @Override + public PointValues.Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { + long min = sortableBytesToLong(minPackedValue); + long max = sortableBytesToLong(maxPackedValue); + + if (min > maxValueAsLong || max < minValueAsLong) { + // 1. cmp ==0 and pruning==Pruning.GREATER_THAN_OR_EQUAL_TO : if the sort is + // ascending then maxValueAsLong is bottom's next less value, so it is competitive + // 2. cmp ==0 and pruning==Pruning.GREATER_THAN: maxValueAsLong equals to + // bottom, but there are multiple comparators, so it could be competitive + return PointValues.Relation.CELL_OUTSIDE_QUERY; } - @Override - public PointValues.Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { - long min = sortableBytesToLong(minPackedValue); - long max = sortableBytesToLong(maxPackedValue); - - if (min > maxValueAsLong || max < minValueAsLong) { - // 1. cmp ==0 and pruning==Pruning.GREATER_THAN_OR_EQUAL_TO : if the sort is - // ascending then maxValueAsLong is bottom's next less value, so it is competitive - // 2. cmp ==0 and pruning==Pruning.GREATER_THAN: maxValueAsLong equals to - // bottom, but there are multiple comparators, so it could be competitive - return PointValues.Relation.CELL_OUTSIDE_QUERY; - } - - if (min < minValueAsLong || max > maxValueAsLong) { - return PointValues.Relation.CELL_CROSSES_QUERY; - } - return PointValues.Relation.CELL_INSIDE_QUERY; + if (min < minValueAsLong || max > maxValueAsLong) { + return PointValues.Relation.CELL_CROSSES_QUERY; } - }; + return PointValues.Relation.CELL_INSIDE_QUERY; + } + }; final long threshold = iteratorCost >>> 3; - if (PointValues.isEstimatedPointCountGreaterThanOrEqualTo( - visitor, getPointTree(), threshold)) { + if (PointValues.isEstimatedPointCountGreaterThanOrEqualTo(visitor, getPointTree(), threshold)) { // the new range is not selective enough to be worth materializing, it doesn't reduce number // of docs at least 8x updateSkipInterval(false); if (pointValues.getDocCount() < iteratorCost) { // Use the set of doc with values to help drive iteration - competitiveIterator.update( - leafComparator.getNumericDocValues(leafComparator.context, field)); + competitiveIterator.update(leafComparator.getNumericDocValues(leafComparator.context, field)); iteratorCost = pointValues.getDocCount(); } return; @@ -495,25 +480,22 @@ private class DVSkipperCompetitiveDISIBuilder extends CompetitiveDISIBuilder { private final DocValuesSkipper skipper; private final TwoPhaseIterator innerTwoPhase; - DVSkipperCompetitiveDISIBuilder( - DocValuesSkipper skipper, NumericLeafComparator leafComparator) throws IOException { + DVSkipperCompetitiveDISIBuilder(DocValuesSkipper skipper, NumericLeafComparator leafComparator) throws IOException { super(leafComparator); this.skipper = skipper; - NumericDocValues docValues = - leafComparator.getNumericDocValues(leafComparator.context, field); - innerTwoPhase = - new TwoPhaseIterator(docValues) { - @Override - public boolean matches() throws IOException { - final long value = docValues.longValue(); - return value >= minValueAsLong && value <= maxValueAsLong; - } + NumericDocValues docValues = leafComparator.getNumericDocValues(leafComparator.context, field); + innerTwoPhase = new TwoPhaseIterator(docValues) { + @Override + public boolean matches() throws IOException { + final long value = docValues.longValue(); + return value >= minValueAsLong && value <= maxValueAsLong; + } - @Override - public float matchCost() { - return 2; // 2 comparisons - } - }; + @Override + public float matchCost() { + return 2; // 2 comparisons + } + }; postInitializeCompetitiveIterator(); } @@ -539,10 +521,8 @@ void postInitializeCompetitiveIterator() { @Override protected void doUpdateCompetitiveIterator() { - TwoPhaseIterator twoPhaseIterator = - new DocValuesRangeIterator(innerTwoPhase, skipper, minValueAsLong, maxValueAsLong, false); + TwoPhaseIterator twoPhaseIterator = new DocValuesRangeIterator(innerTwoPhase, skipper, minValueAsLong, maxValueAsLong, false); competitiveIterator.update(TwoPhaseIterator.asDocIdSetIterator(twoPhaseIterator)); } } } - diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java index 2f9c5767b1a57..d7ce698d0c7af 100644 --- a/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java @@ -70,4 +70,3 @@ public int docIDRunEnd() throws IOException { } } } - diff --git a/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java b/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java index e53dab61c4887..7dd91f6594cce 100644 --- a/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java +++ b/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java @@ -214,9 +214,7 @@ public void testHostnameTimestampSortConfig() throws Exception { assertOrder(backingIndex, orderedDocs); SearchRequest searchRequest = new SearchRequest(dataStreamName); - searchRequest.source().sort("@timestamp", SortOrder.DESC).query( - new TermQueryBuilder("host.name", "aaa") - ); + searchRequest.source().sort("@timestamp", SortOrder.DESC).query(new TermQueryBuilder("host.name", "aaa")); var response = client().search(searchRequest).get(); assertEquals(4, response.getHits().getHits().length); response.decRef(); diff --git a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java index f8aed69688e81..ea3e4df43d615 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java +++ b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java @@ -523,8 +523,6 @@ protected boolean supportsBulkLongBlockReading() { @Override protected List getSortShortcutSupport() { - return List.of( - new SortShortcutSupport(this::minimalMapping, this::writeField, true) - ); + return List.of(new SortShortcutSupport(this::minimalMapping, this::writeField, true)); } } From 3a933b4db12ec89c2005be2abbad2c513811a113 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 3 Nov 2025 17:05:23 +0000 Subject: [PATCH 12/23] spotless --- .../fielddata/IndexNumericFieldData.java | 4 +- .../SecondarySortIterator.java | 2 +- .../lucene/comparators/XLongComparator.java | 6 +- .../comparators/XNumericComparator.java | 176 ++++++++---------- .../XUpdateableDocIdSetIterator.java | 1 - .../xpack/logsdb/LogsdbSortConfigIT.java | 4 +- .../UnsignedLongFieldMapperTests.java | 4 +- 7 files changed, 86 insertions(+), 111 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java index 77999b221f940..503cd1a29775e 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java @@ -176,8 +176,8 @@ public SortField indexSortField(IndexVersion indexCreatedVersion, Object missing // we introduced INT sort type in 8.19 and from 9.1 || indexCreatedVersion.onOrAfter(IndexVersions.INDEX_INT_SORT_INT_TYPE) || indexCreatedVersion.between(IndexVersions.INDEX_INT_SORT_INT_TYPE_8_19, UPGRADE_TO_LUCENE_10_0_0)) { - return sortField; - } + return sortField; + } if ((sortField instanceof SortedNumericSortField) == false) { return sortField; } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java index 818126239f83e..405cc58080751 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIterator.java @@ -90,7 +90,7 @@ public int advance(int target) throws IOException { int upTo = valueFieldUpTo; if (maxValue < this.minValue) { // We've moved past the end of the valid values in the secondary sort field - // for this primary value. Advance the primary skipper to find the starting point + // for this primary value. Advance the primary skipper to find the starting point // for the next primary value, where the secondary field values will have reset primaryFieldSkipper.advance(target); primaryFieldUpTo = primaryFieldSkipper.maxDocID(0); diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java index 6caa94382a6bd..24bdf921efb16 100644 --- a/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XLongComparator.java @@ -23,8 +23,7 @@ public class XLongComparator extends XNumericComparator { protected long topValue; protected long bottom; - public XLongComparator( - int numHits, String field, Long missingValue, boolean reverse, Pruning pruning) { + public XLongComparator(int numHits, String field, Long missingValue, boolean reverse, Pruning pruning) { super(field, missingValue != null ? missingValue : 0L, reverse, pruning, Long.BYTES); values = new long[numHits]; } @@ -68,7 +67,8 @@ public LongLeafComparator(LeafReaderContext context) throws IOException { } @Override - protected XNumericComparator.CompetitiveDISIBuilder buildCompetitiveDISIBuilder(LeafReaderContext context) throws IOException { + protected XNumericComparator.CompetitiveDISIBuilder buildCompetitiveDISIBuilder(LeafReaderContext context) + throws IOException { return super.buildCompetitiveDISIBuilder(context); } diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java index 29eaa221c8002..040f6a769752b 100644 --- a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java @@ -55,8 +55,7 @@ public abstract class XNumericComparator extends FieldComparat protected boolean queueFull; protected Pruning pruning; - protected XNumericComparator( - String field, T missingValue, boolean reverse, Pruning pruning, int bytesCount) { + protected XNumericComparator(String field, T missingValue, boolean reverse, Pruning pruning, int bytesCount) { this.field = field; this.missingValue = missingValue; this.missingValueAsLong = missingValueAsComparableLong(); @@ -129,8 +128,7 @@ protected CompetitiveDISIBuilder buildCompetitiveDISIBuilder(LeafReaderContext c * @return numeric doc values for the field in this segment. * @throws IOException If there is a low-level I/O error */ - protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) - throws IOException { + protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) throws IOException { return DocValues.getNumeric(context.reader(), field); } @@ -217,8 +215,7 @@ final void updateCompetitiveIterator() throws IOException { updateCounter++; // Start sampling if we get called too much - if (updateCounter > 256 - && (updateCounter & (currentSkipInterval - 1)) != currentSkipInterval - 1) { + if (updateCounter > 256 && (updateCounter & (currentSkipInterval - 1)) != currentSkipInterval - 1) { return; } @@ -262,18 +259,12 @@ private void encodeBottom() { private void encodeTop() { if (reverse == false) { minValueAsLong = leafComparator.topAsComparableLong(); - if (singleSort - && pruning == Pruning.GREATER_THAN_OR_EQUAL_TO - && queueFull - && minValueAsLong != Long.MAX_VALUE) { + if (singleSort && pruning == Pruning.GREATER_THAN_OR_EQUAL_TO && queueFull && minValueAsLong != Long.MAX_VALUE) { minValueAsLong++; } } else { maxValueAsLong = leafComparator.topAsComparableLong(); - if (singleSort - && pruning == Pruning.GREATER_THAN_OR_EQUAL_TO - && queueFull - && maxValueAsLong != Long.MIN_VALUE) { + if (singleSort && pruning == Pruning.GREATER_THAN_OR_EQUAL_TO && queueFull && maxValueAsLong != Long.MIN_VALUE) { maxValueAsLong--; } } @@ -286,10 +277,9 @@ boolean isMissingValueCompetitive() { int result = Long.compare(missingValueAsLong, leafComparator.bottomAsComparableLong()); // in reverse (desc) sort missingValue is competitive when it's greater or equal to bottom, // in asc sort missingValue is competitive when it's smaller or equal to bottom - final boolean competitive = - reverse - ? (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO ? result > 0 : result >= 0) - : (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO ? result < 0 : result <= 0); + final boolean competitive = reverse + ? (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO ? result > 0 : result >= 0) + : (pruning == Pruning.GREATER_THAN_OR_EQUAL_TO ? result < 0 : result <= 0); if (competitive == false) { return false; } @@ -323,12 +313,10 @@ private class PointsCompetitiveDISIBuilder extends CompetitiveDISIBuilder { 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"); + "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"); + throw new IllegalArgumentException("Field " + field + " is indexed with multiple dimensions, sorting is not supported"); } else if (info.getPointNumBytes() != bytesCount) { throw new IllegalArgumentException( "Field " @@ -338,7 +326,8 @@ private class PointsCompetitiveDISIBuilder extends CompetitiveDISIBuilder { + " bytes per dimension, but " + this + " expected " - + bytesCount); + + bytesCount + ); } this.pointValues = pointValues; postInitializeCompetitiveIterator(); @@ -348,8 +337,7 @@ private class PointsCompetitiveDISIBuilder extends CompetitiveDISIBuilder { void setScorer(Scorable scorer) throws IOException { if (iteratorCost == -1) { if (scorer instanceof Scorer) { - iteratorCost = - ((Scorer) scorer).iterator().cost(); // starting iterator cost is the scorer's cost + iteratorCost = ((Scorer) scorer).iterator().cost(); // starting iterator cost is the scorer's cost } else { iteratorCost = maxDoc; } @@ -384,78 +372,75 @@ void postInitializeCompetitiveIterator() throws IOException { @Override protected void doUpdateCompetitiveIterator() throws IOException { DocIdSetBuilder result = new DocIdSetBuilder(maxDoc); - PointValues.IntersectVisitor visitor = - new PointValues.IntersectVisitor() { - DocIdSetBuilder.BulkAdder adder; + PointValues.IntersectVisitor visitor = new PointValues.IntersectVisitor() { + DocIdSetBuilder.BulkAdder adder; - @Override - public void grow(int count) { - adder = result.grow(count); - } + @Override + public void grow(int count) { + adder = result.grow(count); + } - @Override - public void visit(int docID) { - if (docID <= maxDocVisited) { - return; // Already visited or skipped - } - adder.add(docID); + @Override + public void visit(int docID) { + if (docID <= maxDocVisited) { + return; // Already visited or skipped } + adder.add(docID); + } - @Override - public void visit(int docID, byte[] packedValue) { - if (docID <= maxDocVisited) { - return; // already visited or skipped - } - long l = sortableBytesToLong(packedValue); - if (l >= minValueAsLong && l <= maxValueAsLong) { - adder.add(docID); // doc is competitive - } + @Override + public void visit(int docID, byte[] packedValue) { + if (docID <= maxDocVisited) { + return; // already visited or skipped + } + long l = sortableBytesToLong(packedValue); + if (l >= minValueAsLong && l <= maxValueAsLong) { + adder.add(docID); // doc is competitive } + } - @Override - public void visit(DocIdSetIterator iterator) throws IOException { - if (iterator.advance(maxDocVisited + 1) != DocIdSetIterator.NO_MORE_DOCS) { - adder.add(iterator.docID()); - adder.add(iterator); - } + @Override + public void visit(DocIdSetIterator iterator) throws IOException { + if (iterator.advance(maxDocVisited + 1) != DocIdSetIterator.NO_MORE_DOCS) { + adder.add(iterator.docID()); + adder.add(iterator); } + } - @Override - public void visit(IntsRef ref) { - adder.add(ref, maxDocVisited + 1); + @Override + public void visit(IntsRef ref) { + adder.add(ref, maxDocVisited + 1); + } + + @Override + public PointValues.Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { + long min = sortableBytesToLong(minPackedValue); + long max = sortableBytesToLong(maxPackedValue); + + if (min > maxValueAsLong || max < minValueAsLong) { + // 1. cmp ==0 and pruning==Pruning.GREATER_THAN_OR_EQUAL_TO : if the sort is + // ascending then maxValueAsLong is bottom's next less value, so it is competitive + // 2. cmp ==0 and pruning==Pruning.GREATER_THAN: maxValueAsLong equals to + // bottom, but there are multiple comparators, so it could be competitive + return PointValues.Relation.CELL_OUTSIDE_QUERY; } - @Override - public PointValues.Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { - long min = sortableBytesToLong(minPackedValue); - long max = sortableBytesToLong(maxPackedValue); - - if (min > maxValueAsLong || max < minValueAsLong) { - // 1. cmp ==0 and pruning==Pruning.GREATER_THAN_OR_EQUAL_TO : if the sort is - // ascending then maxValueAsLong is bottom's next less value, so it is competitive - // 2. cmp ==0 and pruning==Pruning.GREATER_THAN: maxValueAsLong equals to - // bottom, but there are multiple comparators, so it could be competitive - return PointValues.Relation.CELL_OUTSIDE_QUERY; - } - - if (min < minValueAsLong || max > maxValueAsLong) { - return PointValues.Relation.CELL_CROSSES_QUERY; - } - return PointValues.Relation.CELL_INSIDE_QUERY; + if (min < minValueAsLong || max > maxValueAsLong) { + return PointValues.Relation.CELL_CROSSES_QUERY; } - }; + return PointValues.Relation.CELL_INSIDE_QUERY; + } + }; final long threshold = iteratorCost >>> 3; - if (PointValues.isEstimatedPointCountGreaterThanOrEqualTo( - visitor, getPointTree(), threshold)) { + if (PointValues.isEstimatedPointCountGreaterThanOrEqualTo(visitor, getPointTree(), threshold)) { // the new range is not selective enough to be worth materializing, it doesn't reduce number // of docs at least 8x updateSkipInterval(false); if (pointValues.getDocCount() < iteratorCost) { // Use the set of doc with values to help drive iteration - competitiveIterator.update( - leafComparator.getNumericDocValues(leafComparator.context, field)); + competitiveIterator.update(leafComparator.getNumericDocValues(leafComparator.context, field)); iteratorCost = pointValues.getDocCount(); } return; @@ -495,25 +480,22 @@ private class DVSkipperCompetitiveDISIBuilder extends CompetitiveDISIBuilder { private final DocValuesSkipper skipper; private final TwoPhaseIterator innerTwoPhase; - DVSkipperCompetitiveDISIBuilder( - DocValuesSkipper skipper, NumericLeafComparator leafComparator) throws IOException { + DVSkipperCompetitiveDISIBuilder(DocValuesSkipper skipper, NumericLeafComparator leafComparator) throws IOException { super(leafComparator); this.skipper = skipper; - NumericDocValues docValues = - leafComparator.getNumericDocValues(leafComparator.context, field); - innerTwoPhase = - new TwoPhaseIterator(docValues) { - @Override - public boolean matches() throws IOException { - final long value = docValues.longValue(); - return value >= minValueAsLong && value <= maxValueAsLong; - } + NumericDocValues docValues = leafComparator.getNumericDocValues(leafComparator.context, field); + innerTwoPhase = new TwoPhaseIterator(docValues) { + @Override + public boolean matches() throws IOException { + final long value = docValues.longValue(); + return value >= minValueAsLong && value <= maxValueAsLong; + } - @Override - public float matchCost() { - return 2; // 2 comparisons - } - }; + @Override + public float matchCost() { + return 2; // 2 comparisons + } + }; postInitializeCompetitiveIterator(); } @@ -539,10 +521,8 @@ void postInitializeCompetitiveIterator() { @Override protected void doUpdateCompetitiveIterator() { - TwoPhaseIterator twoPhaseIterator = - new DocValuesRangeIterator(innerTwoPhase, skipper, minValueAsLong, maxValueAsLong, false); + TwoPhaseIterator twoPhaseIterator = new DocValuesRangeIterator(innerTwoPhase, skipper, minValueAsLong, maxValueAsLong, false); competitiveIterator.update(TwoPhaseIterator.asDocIdSetIterator(twoPhaseIterator)); } } } - diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java index 2f9c5767b1a57..d7ce698d0c7af 100644 --- a/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XUpdateableDocIdSetIterator.java @@ -70,4 +70,3 @@ public int docIDRunEnd() throws IOException { } } } - diff --git a/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java b/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java index e53dab61c4887..7dd91f6594cce 100644 --- a/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java +++ b/x-pack/plugin/logsdb/src/internalClusterTest/java/org/elasticsearch/xpack/logsdb/LogsdbSortConfigIT.java @@ -214,9 +214,7 @@ public void testHostnameTimestampSortConfig() throws Exception { assertOrder(backingIndex, orderedDocs); SearchRequest searchRequest = new SearchRequest(dataStreamName); - searchRequest.source().sort("@timestamp", SortOrder.DESC).query( - new TermQueryBuilder("host.name", "aaa") - ); + searchRequest.source().sort("@timestamp", SortOrder.DESC).query(new TermQueryBuilder("host.name", "aaa")); var response = client().search(searchRequest).get(); assertEquals(4, response.getHits().getHits().length); response.decRef(); diff --git a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java index f8aed69688e81..ea3e4df43d615 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java +++ b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java @@ -523,8 +523,6 @@ protected boolean supportsBulkLongBlockReading() { @Override protected List getSortShortcutSupport() { - return List.of( - new SortShortcutSupport(this::minimalMapping, this::writeField, true) - ); + return List.of(new SortShortcutSupport(this::minimalMapping, this::writeField, true)); } } From 79bf9b28864b53eb62dac9f75bbf05f78fda9bd3 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 4 Nov 2025 13:43:24 +0000 Subject: [PATCH 13/23] tests --- .../java/org/elasticsearch/common/lucene/Lucene.java | 7 ++----- .../fieldcomparator/DoubleValuesComparatorSource.java | 2 +- .../fieldcomparator/FloatValuesComparatorSource.java | 2 +- .../fieldcomparator/LongValuesComparatorSource.java | 9 +++++++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java index 0ef0e3ffa28c8..78ae54f0a8b0d 100644 --- a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java +++ b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java @@ -634,10 +634,10 @@ private static SortField rewriteMergeSortField(SortField sortField) { SortField newSortField = new SortField(sortField.getField(), SortField.Type.STRING, sortField.getReverse()); newSortField.setMissingValue(sortField.getMissingValue()); return newSortField; - } else if (sortField.getClass() == SortedNumericSortField.class) { + } else if (sortField instanceof SortedNumericSortField snsf) { SortField newSortField = new SortField( sortField.getField(), - ((SortedNumericSortField) sortField).getNumericType(), + snsf.getNumericType(), sortField.getReverse() ); newSortField.setMissingValue(sortField.getMissingValue()); @@ -651,9 +651,6 @@ private static SortField rewriteMergeSortField(SortField sortField) { static void writeSortField(StreamOutput out, SortField sortField) throws IOException { sortField = rewriteMergeSortField(sortField); - if (sortField.getClass() != SortField.class) { - throw new IllegalArgumentException("Cannot serialize SortField impl [" + sortField + "]"); - } out.writeOptionalString(sortField.getField()); if (sortField.getComparatorSource() != null) { IndexFieldData.XFieldComparatorSource comparatorSource = (IndexFieldData.XFieldComparatorSource) sortField diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java index c5fcb0207ce4d..d8cfc099b5008 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java @@ -80,7 +80,7 @@ public FieldComparator newComparator(String fieldname, int numHits, Pruning e final double dMissingValue = (Double) missingObject(missingValue, reversed); // NOTE: it's important to pass null as a missing value in the constructor so that // the comparator doesn't check docsWithField since we replace missing values in select() - return new DoubleComparator(numHits, null, null, reversed, Pruning.NONE) { + return new DoubleComparator(numHits, fieldname, null, reversed, enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new DoubleLeafComparator(context) { diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java index 79f1fdb25a0a6..53801dca99f67 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java @@ -73,7 +73,7 @@ public FieldComparator newComparator(String fieldname, int numHits, Pruning e final float fMissingValue = (Float) missingObject(missingValue, reversed); // NOTE: it's important to pass null as a missing value in the constructor so that // the comparator doesn't check docsWithField since we replace missing values in select() - return new FloatComparator(numHits, null, null, reversed, Pruning.NONE) { + return new FloatComparator(numHits, fieldname, null, reversed, enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new FloatLeafComparator(context) { diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index 1db4ef1feb078..a0772a82f564c 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java @@ -202,6 +202,9 @@ public Object missingObject(Object missingValue, boolean reversed) { protected static NumericDocValues wrap(LongValues longValues) { return new NumericDocValues() { + + int doc = -1; + @Override public long longValue() throws IOException { return longValues.longValue(); @@ -209,17 +212,18 @@ public long longValue() throws IOException { @Override public boolean advanceExact(int target) throws IOException { + doc = target; return longValues.advanceExact(target); } @Override public int docID() { - throw new UnsupportedOperationException(); + return doc; } @Override public int nextDoc() throws IOException { - throw new UnsupportedOperationException(); + return advance(doc + 1); } @Override @@ -228,6 +232,7 @@ public int advance(int target) throws IOException { // always return `true` from `advanceExact()` boolean hasValue = longValues.advanceExact(target); assert hasValue : "LongValuesComparatorSource#wrap called with a LongValues that has missing values"; + doc = target; return target; } From c3b270c81ee0ab6d8afb2a1f51328e015db4f3d6 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Tue, 4 Nov 2025 13:49:53 +0000 Subject: [PATCH 14/23] [CI] Auto commit changes from spotless --- .../main/java/org/elasticsearch/common/lucene/Lucene.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java index 78ae54f0a8b0d..517077bfe5401 100644 --- a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java +++ b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java @@ -635,11 +635,7 @@ private static SortField rewriteMergeSortField(SortField sortField) { newSortField.setMissingValue(sortField.getMissingValue()); return newSortField; } else if (sortField instanceof SortedNumericSortField snsf) { - SortField newSortField = new SortField( - sortField.getField(), - snsf.getNumericType(), - sortField.getReverse() - ); + SortField newSortField = new SortField(sortField.getField(), snsf.getNumericType(), sortField.getReverse()); newSortField.setMissingValue(sortField.getMissingValue()); return newSortField; } else if (sortField.getClass() == ShardDocSortField.class) { From dd1f9946a54c99a72b5a4e303e36cc0e470c720c Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 4 Nov 2025 15:12:53 +0000 Subject: [PATCH 15/23] need to add iterators back in for pruning - follow up --- .../fieldcomparator/DoubleValuesComparatorSource.java | 3 ++- .../fieldcomparator/FloatValuesComparatorSource.java | 3 ++- .../HalfFloatValuesComparatorSource.java | 3 ++- .../index/mapper/DoubleFieldMapperTests.java | 10 ++++++++++ .../index/mapper/FloatFieldMapperTests.java | 9 +++++++++ .../index/mapper/HalfFloatFieldMapperTests.java | 10 ++++++++++ 6 files changed, 35 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java index d8cfc099b5008..a1f2b30332630 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java @@ -80,7 +80,8 @@ public FieldComparator newComparator(String fieldname, int numHits, Pruning e final double dMissingValue = (Double) missingObject(missingValue, reversed); // NOTE: it's important to pass null as a missing value in the constructor so that // the comparator doesn't check docsWithField since we replace missing values in select() - return new DoubleComparator(numHits, fieldname, null, reversed, enableSkipping) { + // TODO we can re-enable pruning here if we allow NumericDoubleValues to expose an iterator + return new DoubleComparator(numHits, fieldname, null, reversed, Pruning.NONE) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new DoubleLeafComparator(context) { diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java index 53801dca99f67..6e30c94411953 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java @@ -73,7 +73,8 @@ public FieldComparator newComparator(String fieldname, int numHits, Pruning e final float fMissingValue = (Float) missingObject(missingValue, reversed); // NOTE: it's important to pass null as a missing value in the constructor so that // the comparator doesn't check docsWithField since we replace missing values in select() - return new FloatComparator(numHits, fieldname, null, reversed, enableSkipping) { + // TODO we can re-enable pruning here if we allow NumericDoubleValues to expose an iterator + return new FloatComparator(numHits, fieldname, null, reversed, Pruning.NONE) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new FloatLeafComparator(context) { diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java index ade3f5ccc5a3a..aa2c7c9bdef5d 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java @@ -39,7 +39,8 @@ public FieldComparator newComparator(String fieldname, int numHits, Pruning e final float fMissingValue = (Float) missingObject(missingValue, reversed); // NOTE: it's important to pass null as a missing value in the constructor so that // the comparator doesn't check docsWithField since we replace missing values in select() - return new HalfFloatComparator(numHits, fieldname, null, reversed, enableSkipping) { + // TODO we can re-enable pruning here if we allow NumericDoubleValues to expose an iterator + return new HalfFloatComparator(numHits, fieldname, null, reversed, Pruning.NONE) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new HalfFloatLeafComparator(context) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java index c56ad3a1e56eb..10495d2d30568 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.index.mapper; +import org.elasticsearch.index.IndexVersion; import org.elasticsearch.script.DoubleFieldScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; @@ -164,4 +165,13 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) protected SyntheticSourceSupport syntheticSourceSupportForKeepTests(boolean ignoreMalformed, Mapper.SourceKeepMode sourceKeepMode) { return new NumberSyntheticSourceSupportForKeepTests(Number::doubleValue, ignoreMalformed, sourceKeepMode); } + + @Override + protected List getSortShortcutSupport() { + return List.of( + // TODO enable pruning here + new SortShortcutSupport(this::minimalMapping, this::writeField, false), + new SortShortcutSupport(IndexVersion.fromId(5000099), this::minimalMapping, this::writeField, false) + ); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FloatFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FloatFieldMapperTests.java index 148c7b1b86e44..7f9a242fac1fb 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FloatFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FloatFieldMapperTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.index.mapper; +import org.elasticsearch.index.IndexVersion; import org.elasticsearch.xcontent.XContentBuilder; import org.junit.AssumptionViolatedException; @@ -69,4 +70,12 @@ protected SyntheticSourceSupport syntheticSourceSupportForKeepTests(boolean igno protected IngestScriptSupport ingestScriptSupport() { throw new AssumptionViolatedException("not supported"); } + + @Override + protected List getSortShortcutSupport() { + return List.of( + new SortShortcutSupport(this::minimalMapping, this::writeField, false), + new SortShortcutSupport(IndexVersion.fromId(5000099), this::minimalMapping, this::writeField, false) + ); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/HalfFloatFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/HalfFloatFieldMapperTests.java index 8264f53661320..329379a9c56cb 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/HalfFloatFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/HalfFloatFieldMapperTests.java @@ -10,6 +10,7 @@ package org.elasticsearch.index.mapper; import org.apache.lucene.sandbox.document.HalfFloatPoint; +import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType; import org.elasticsearch.xcontent.XContentBuilder; import org.junit.AssumptionViolatedException; @@ -73,4 +74,13 @@ protected IngestScriptSupport ingestScriptSupport() { protected boolean supportsBulkDoubleBlockReading() { return true; } + + @Override + protected List getSortShortcutSupport() { + return List.of( + // TODO enable pruning here + new SortShortcutSupport(this::minimalMapping, this::writeField, false), + new SortShortcutSupport(IndexVersion.fromId(5000099), this::minimalMapping, this::writeField, false) + ); + } } From 4fa6f8e0579ce74bc48e4623a405ae122b7785af Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 4 Nov 2025 16:25:02 +0000 Subject: [PATCH 16/23] dont' advance past maxdoc --- .../fieldcomparator/LongValuesComparatorSource.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index a0772a82f564c..b6deaf0adffda 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java @@ -112,7 +112,7 @@ public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws I return new LongLeafComparator(context) { @Override protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) throws IOException { - return wrap(getLongValues(context, lMissingValue)); + return wrap(getLongValues(context, lMissingValue), context.reader().maxDoc()); } @Override @@ -200,7 +200,7 @@ public Object missingObject(Object missingValue, boolean reversed) { return super.missingObject(missingValue, reversed); } - protected static NumericDocValues wrap(LongValues longValues) { + protected static NumericDocValues wrap(LongValues longValues, int maxDoc) { return new NumericDocValues() { int doc = -1; @@ -228,6 +228,9 @@ public int nextDoc() throws IOException { @Override public int advance(int target) throws IOException { + if (target > maxDoc) { + return doc = NO_MORE_DOCS; + } // All documents are guaranteed to have a value, as all invocations of getLongValues // always return `true` from `advanceExact()` boolean hasValue = longValues.advanceExact(target); From 12492860d01a1348ceadd67ee569097cd00d27f8 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 4 Nov 2025 16:35:47 +0000 Subject: [PATCH 17/23] compilation --- .../fielddata/fieldcomparator/IntValuesComparatorSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/IntValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/IntValuesComparatorSource.java index 544b933772065..d2fdd2f2900a6 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/IntValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/IntValuesComparatorSource.java @@ -60,7 +60,7 @@ public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws I return new IntLeafComparator(context) { @Override protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) throws IOException { - return wrap(getLongValues(context, iMissingValue)); + return wrap(getLongValues(context, iMissingValue), context.reader().maxDoc()); } }; } From 0e0e392961eab98f1099f7db7ae3e2827e26c9e9 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 5 Nov 2025 10:42:03 +0100 Subject: [PATCH 18/23] added logic from lucene pr --- .../lucene/comparators/XNumericComparator.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java index 040f6a769752b..2ac33da3a1e8c 100644 --- a/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java +++ b/server/src/main/java/org/elasticsearch/lucene/comparators/XNumericComparator.java @@ -357,7 +357,12 @@ protected int docCount() { * @throws IOException i/o exception while fetching min and max values from point values */ void postInitializeCompetitiveIterator() throws IOException { - if (queueFull) { + if (queueFull && hitsThresholdReached) { + // if some documents have missing doc values, check that missing values prohibits + // optimization + if (docCount() < maxDoc && isMissingValueCompetitive()) { + return; + } long bottom = leafComparator.bottomAsComparableLong(); long minValue = sortableBytesToLong(pointValues.getMinPackedValue()); long maxValue = sortableBytesToLong(pointValues.getMaxPackedValue()); @@ -509,7 +514,12 @@ protected int docCount() { * iterator as competitive iterator. */ void postInitializeCompetitiveIterator() { - if (queueFull) { + if (queueFull && hitsThresholdReached) { + // if some documents have missing doc values, check that missing values prohibits + // optimization + if (docCount() < maxDoc && isMissingValueCompetitive()) { + return; + } long bottom = leafComparator.bottomAsComparableLong(); if (reverse == false && bottom < skipper.minValue()) { competitiveIterator.update(DocIdSetIterator.empty()); From cb9a7f54a15a9355709932381e01531de97e3b1c Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 5 Nov 2025 09:57:19 +0000 Subject: [PATCH 19/23] fencepost --- .../fielddata/fieldcomparator/LongValuesComparatorSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index b6deaf0adffda..fcd66789059e6 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java @@ -228,7 +228,7 @@ public int nextDoc() throws IOException { @Override public int advance(int target) throws IOException { - if (target > maxDoc) { + if (target >= maxDoc) { return doc = NO_MORE_DOCS; } // All documents are guaranteed to have a value, as all invocations of getLongValues From d0a2218b3d334052e743efcab672a4ead8bff046 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 5 Nov 2025 10:01:18 +0000 Subject: [PATCH 20/23] only run accelerator on dense fields --- .../fieldcomparator/LongValuesComparatorSource.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index fcd66789059e6..82830d5bfe3bd 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java @@ -109,10 +109,11 @@ public FieldComparator newComparator(String fieldname, int numHits, Pruning e return new XLongComparator(numHits, fieldname, null, reversed, enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { + final int maxDoc = context.reader().maxDoc(); return new LongLeafComparator(context) { @Override protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) throws IOException { - return wrap(getLongValues(context, lMissingValue), context.reader().maxDoc()); + return wrap(getLongValues(context, lMissingValue), maxDoc); } @Override @@ -131,7 +132,7 @@ protected XNumericComparator.CompetitiveDISIBuilder buildCompetitiveDISIBu } DocValuesSkipper skipper = context.reader().getDocValuesSkipper(field); DocValuesSkipper primaryFieldSkipper = context.reader().getDocValuesSkipper(sortFields[0].getField()); - if (primaryFieldSkipper == null) { + if (primaryFieldSkipper == null || skipper.docCount() != maxDoc || primaryFieldSkipper.docCount() != maxDoc) { return super.buildCompetitiveDISIBuilder(context); } return new CompetitiveDISIBuilder(this) { From 2dc921ec6d7800aff43630c9fa3715f08bd8564e Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 6 Nov 2025 10:13:13 +0000 Subject: [PATCH 21/23] Revert subclassing nonsense --- .../elasticsearch/index/IndexSortConfig.java | 2 +- .../index/fielddata/IndexFieldData.java | 12 --- .../fielddata/IndexNumericFieldData.java | 84 +++---------------- .../search/sort/FieldSortBuilder.java | 2 +- .../index/mapper/DoubleFieldMapperTests.java | 3 +- .../index/mapper/FloatFieldMapperTests.java | 2 +- 6 files changed, 14 insertions(+), 91 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/IndexSortConfig.java b/server/src/main/java/org/elasticsearch/index/IndexSortConfig.java index 37742a4f5104d..fd445d470837c 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexSortConfig.java +++ b/server/src/main/java/org/elasticsearch/index/IndexSortConfig.java @@ -375,7 +375,7 @@ public Sort buildIndexSort( if (fieldData == null) { throw new IllegalArgumentException("docvalues not found for index sort field:[" + sortSpec.field + "]"); } - sortFields[i] = fieldData.indexSortField(this.indexCreatedVersion, sortSpec.missingValue, mode, reverse); + sortFields[i] = fieldData.sortField(this.indexCreatedVersion, sortSpec.missingValue, mode, null, reverse); validateIndexSortField(sortFields[i]); } return new Sort(sortFields); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldData.java index 13bffa6dff0f1..5f6cc93063d87 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldData.java @@ -83,18 +83,6 @@ default SortField sortField( */ SortField sortField(@Nullable Object missingValue, MultiValueMode sortMode, Nested nested, boolean reverse); - /** - * Returns the {@link SortField} to use for IndexSorting, if supported - */ - default SortField indexSortField( - IndexVersion indexCreatedVersion, - @Nullable Object missingValue, - MultiValueMode sortMode, - boolean reverse - ) { - return sortField(indexCreatedVersion, missingValue, sortMode, null, reverse); - } - /** * Build a sort implementation specialized for aggregations. */ diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java index 503cd1a29775e..53a8bcf2776aa 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java @@ -9,8 +9,6 @@ package org.elasticsearch.index.fielddata; -import org.apache.lucene.search.FieldComparator; -import org.apache.lucene.search.Pruning; import org.apache.lucene.search.SortField; import org.apache.lucene.search.SortedNumericSelector; import org.apache.lucene.search.SortedNumericSortField; @@ -88,7 +86,6 @@ public final ValuesSourceType getValuesSourceType() { * match the field's numericType. */ public final SortField sortField( - boolean isIndexSort, NumericType targetNumericType, Object missingValue, MultiValueMode sortMode, @@ -107,34 +104,18 @@ public final SortField sortField( boolean requiresCustomComparator = nested != null || (sortMode != MultiValueMode.MAX && sortMode != MultiValueMode.MIN) || targetNumericType != getNumericType(); - boolean canUseOptimizedSort = canUseOptimizedSort(indexType()); if (sortRequiresCustomComparator() || requiresCustomComparator) { - if (isIndexSort) { - return new SortField(getFieldName(), source, reverse); - } - return new SortField(getFieldName(), source, reverse) { - @Override - public FieldComparator getComparator(int numHits, Pruning pruning) { - return super.getComparator(numHits, canUseOptimizedSort == false || requiresCustomComparator ? Pruning.NONE : pruning); - } - }; + SortField sortField = new SortField(getFieldName(), source, reverse); + sortField.setOptimizeSortWithPoints(requiresCustomComparator == false && canUseOptimizedSort(indexType())); + return sortField; } SortedNumericSelector.Type selectorType = sortMode == MultiValueMode.MAX ? SortedNumericSelector.Type.MAX : SortedNumericSelector.Type.MIN; - if (isIndexSort) { - SortField sortField = new SortedNumericSortField(getFieldName(), getNumericType().sortFieldType, reverse, selectorType); - sortField.setMissingValue(source.missingObject(missingValue, reverse)); - return sortField; - } - SortField sortField = new SortedNumericSortField(getFieldName(), getNumericType().sortFieldType, reverse, selectorType) { - @Override - public FieldComparator getComparator(int numHits, Pruning pruning) { - return source.newComparator(getFieldName(), numHits, canUseOptimizedSort == false ? Pruning.NONE : pruning, reverse); - } - }; + SortField sortField = new SortedNumericSortField(getFieldName(), getNumericType().sortFieldType, reverse, selectorType); sortField.setMissingValue(source.missingObject(missingValue, reverse)); + sortField.setOptimizeSortWithPoints(canUseOptimizedSort(indexType())); return sortField; } @@ -157,48 +138,7 @@ private static boolean canUseOptimizedSort(IndexType indexType) { @Override public final SortField sortField(Object missingValue, MultiValueMode sortMode, Nested nested, boolean reverse) { - return sortField(false, getNumericType(), missingValue, sortMode, nested, reverse); - } - - @Override - public SortField indexSortField(IndexVersion indexCreatedVersion, Object missingValue, MultiValueMode sortMode, boolean reverse) { - SortField sortField = sortField(true, getNumericType(), missingValue, sortMode, null, reverse); - if (getNumericType() == NumericType.DATE_NANOSECONDS - && indexCreatedVersion.before(IndexVersions.V_7_14_0) - && missingValue.equals("_last") - && Long.valueOf(0L).equals(sortField.getMissingValue())) { - // 7.14 changed the default missing value of sort on date_nanos, from Long.MIN_VALUE - // to 0L - for compatibility we require to a missing value of MIN_VALUE to allow to - // open the index. - sortField.setMissingValue(Long.MIN_VALUE); - return sortField; - } else if (getNumericType().sortFieldType != SortField.Type.INT - // we introduced INT sort type in 8.19 and from 9.1 - || indexCreatedVersion.onOrAfter(IndexVersions.INDEX_INT_SORT_INT_TYPE) - || indexCreatedVersion.between(IndexVersions.INDEX_INT_SORT_INT_TYPE_8_19, UPGRADE_TO_LUCENE_10_0_0)) { - return sortField; - } - if ((sortField instanceof SortedNumericSortField) == false) { - return sortField; - } - // if the index was created before 8.19, or in 9.0 - // we need to rewrite the sort field to use LONG sort type - - // Rewrite INT sort to LONG sort. - // Before indices used TYPE.LONG for index sorting on integer field, - // and this is stored in their index writer config on disk and can't be modified. - // Now sortField() returns TYPE.INT when sorting on integer field, - // but to support sorting on old indices, we need to rewrite this sort to TYPE.LONG. - XFieldComparatorSource longSource = comparatorSource(NumericType.LONG, missingValue, sortMode, null); - SortedNumericSortField numericSortField = (SortedNumericSortField) sortField; - SortedNumericSortField rewrittenSortField = new SortedNumericSortField( - sortField.getField(), - SortField.Type.LONG, - sortField.getReverse(), - numericSortField.getSelector() - ); - rewrittenSortField.setMissingValue(longSource.missingObject(missingValue, reverse)); - return rewrittenSortField; + return sortField(getNumericType(), missingValue, sortMode, nested, reverse); } @Override @@ -236,21 +176,17 @@ public SortField sortField( // and this is stored in their index writer config on disk and can't be modified. // Now sortField() returns TYPE.INT when sorting on integer field, // but to support sorting on old indices, we need to rewrite this sort to TYPE.LONG. - XFieldComparatorSource longSource = comparatorSource(NumericType.LONG, missingValue, sortMode, nested); SortedNumericSortField numericSortField = (SortedNumericSortField) sortField; SortedNumericSortField rewrittenSortField = new SortedNumericSortField( sortField.getField(), SortField.Type.LONG, sortField.getReverse(), numericSortField.getSelector() - ) { - @Override - public FieldComparator getComparator(int numHits, Pruning pruning) { - // we don't optimize sorting on int field for old indices - return super.getComparator(numHits, Pruning.NONE); - } - }; + ); + XFieldComparatorSource longSource = comparatorSource(NumericType.LONG, missingValue, sortMode, nested); rewrittenSortField.setMissingValue(longSource.missingObject(missingValue, reverse)); + // we don't optimize sorting on int field for old indices + rewrittenSortField.setOptimizeSortWithPoints(false); return rewrittenSortField; } diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index bd12161330ab2..8758aa6dfbe1a 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -355,7 +355,7 @@ public SortFieldAndFormat build(SearchExecutionContext context) throws IOExcepti } IndexNumericFieldData numericFieldData = (IndexNumericFieldData) fieldData; NumericType resolvedType = resolveNumericType(numericType); - field = numericFieldData.sortField(false, resolvedType, missing, localSortMode(), nested, reverse); + field = numericFieldData.sortField(resolvedType, missing, localSortMode(), nested, reverse); isNanosecond = resolvedType == NumericType.DATE_NANOSECONDS; } else { field = fieldData.sortField(context.indexVersionCreated(), missing, localSortMode(), nested, reverse); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java index 10495d2d30568..5a42bd66d0fa0 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java @@ -169,8 +169,7 @@ protected SyntheticSourceSupport syntheticSourceSupportForKeepTests(boolean igno @Override protected List getSortShortcutSupport() { return List.of( - // TODO enable pruning here - new SortShortcutSupport(this::minimalMapping, this::writeField, false), + new SortShortcutSupport(this::minimalMapping, this::writeField, true), new SortShortcutSupport(IndexVersion.fromId(5000099), this::minimalMapping, this::writeField, false) ); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FloatFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FloatFieldMapperTests.java index 7f9a242fac1fb..42183543ccf10 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FloatFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FloatFieldMapperTests.java @@ -74,7 +74,7 @@ protected IngestScriptSupport ingestScriptSupport() { @Override protected List getSortShortcutSupport() { return List.of( - new SortShortcutSupport(this::minimalMapping, this::writeField, false), + new SortShortcutSupport(this::minimalMapping, this::writeField, true), new SortShortcutSupport(IndexVersion.fromId(5000099), this::minimalMapping, this::writeField, false) ); } From 25dbeb4115f5ae9dd294803cf9bbd7a2ee223684 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 6 Nov 2025 15:49:28 +0000 Subject: [PATCH 22/23] Add unit test for SSI --- .../SecondarySortIteratorTests.java | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 server/src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIteratorTests.java diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIteratorTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIteratorTests.java new file mode 100644 index 0000000000000..54c7892b7b2b3 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/SecondarySortIteratorTests.java @@ -0,0 +1,94 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.index.fielddata.fieldcomparator; + +import org.apache.lucene.document.Document; +import org.apache.lucene.document.NumericDocValuesField; +import org.apache.lucene.document.SortedDocValuesField; +import org.apache.lucene.index.DocValuesSkipper; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.search.DocIdSetIterator; +import org.apache.lucene.search.DocValuesRangeIterator; +import org.apache.lucene.search.Sort; +import org.apache.lucene.search.SortField; +import org.apache.lucene.search.TwoPhaseIterator; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.analysis.MockAnalyzer; +import org.apache.lucene.tests.index.RandomIndexWriter; +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; + +public class SecondarySortIteratorTests extends ESTestCase { + + public void testAgainstDocValuesRangeIterator() throws IOException { + + Directory dir = newDirectory(); + Sort indexSort = new Sort(new SortField("hostname", SortField.Type.STRING), new SortField("@timestamp", SortField.Type.LONG, true)); + IndexWriterConfig iwc = new IndexWriterConfig(new MockAnalyzer(random())).setIndexSort(indexSort); + RandomIndexWriter indexWriter = new RandomIndexWriter(random(), dir, iwc); + + int numDocs = atLeast(1000); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + doc.add(SortedDocValuesField.indexedField("hostname", new BytesRef("host1"))); + doc.add(NumericDocValuesField.indexedField("@timestamp", 1_000_000 + i)); + indexWriter.addDocument(doc); + } + + indexWriter.forceMerge(1); + + IndexReader reader = indexWriter.getReader(); + long start = 1_000_400; + long end = 1_000_499; + DocIdSetIterator dvIt = docValuesRangeIterator(reader.leaves().getFirst(), start, end); + DocIdSetIterator ssIt = secondarySortIterator(reader.leaves().getFirst(), start, end); + + // Because the primary sort field has only a single value, we should get exactly the same + // results from the secondary sort iterator as from a standard DVRangeIterator over the + // secondary field + for (int doc = dvIt.nextDoc(); doc != DocIdSetIterator.NO_MORE_DOCS; doc = dvIt.nextDoc()) { + assertEquals(doc, ssIt.nextDoc()); + } + assertEquals(DocIdSetIterator.NO_MORE_DOCS, ssIt.nextDoc()); + + reader.close(); + indexWriter.close(); + dir.close(); + } + + private static DocIdSetIterator secondarySortIterator(LeafReaderContext ctx, long start, long end) throws IOException { + NumericDocValues timeStampDV = ctx.reader().getNumericDocValues("@timestamp"); + DocValuesSkipper primarySkipper = ctx.reader().getDocValuesSkipper("hostname"); + DocValuesSkipper secondarySkipper = ctx.reader().getDocValuesSkipper("@timestamp"); + return new SecondarySortIterator(timeStampDV, secondarySkipper, primarySkipper, start, end); + } + + private static DocIdSetIterator docValuesRangeIterator(LeafReaderContext ctx, long start, long end) throws IOException { + NumericDocValues timeStampDV = ctx.reader().getNumericDocValues("@timestamp"); + TwoPhaseIterator twoPhaseIterator = new TwoPhaseIterator(timeStampDV) { + @Override + public boolean matches() throws IOException { + return timeStampDV.longValue() >= start && timeStampDV.longValue() <= end; + } + + @Override + public float matchCost() { + return 2; + } + }; + DocValuesSkipper skipper = ctx.reader().getDocValuesSkipper("@timestamp"); + return TwoPhaseIterator.asDocIdSetIterator(new DocValuesRangeIterator(twoPhaseIterator, skipper, start, end, false)); + } +} From 3b6d308069285dac354ac7731395c9f5c3cea1a0 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 7 Nov 2025 16:13:32 +0000 Subject: [PATCH 23/23] conflicts --- .../java/org/elasticsearch/index/mapper/MapperTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index ed84bfb3fe56b..7e32e5e2b1c00 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -1773,7 +1773,7 @@ public final void testSortShortcuts() throws IOException { ); }, reader -> { IndexSearcher searcher = newSearcher(reader); - MappedFieldType ft = mapperService.fieldType("field"); + MappedFieldType ft = mapperService.fieldType(sortShortcutSupport.fieldname); SortField sortField = ft.fielddataBuilder(new FieldDataContext("", mapperService.getIndexSettings(), () -> { throw new UnsupportedOperationException(); }, Set::of, MappedFieldType.FielddataOperation.SEARCH))