diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e1fd535c5095..0581ff1ac12dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Addition of Doc values on the GeoShape Field - Addition of GeoShape ValueSource level code interfaces for accessing the DocValues. - Addition of Missing Value feature in the GeoShape Aggregations. +- Enable Point based optimization for custom comparators ([#8168](https://github.com/opensearch-project/OpenSearch/pull/8168)) ### Dependencies - Bump `com.azure:azure-storage-common` from 12.21.0 to 12.21.1 (#7566, #7814) diff --git a/server/src/main/java/org/opensearch/index/fielddata/IndexFieldData.java b/server/src/main/java/org/opensearch/index/fielddata/IndexFieldData.java index 529405c90c9a4..36e6a242ecdec 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/IndexFieldData.java +++ b/server/src/main/java/org/opensearch/index/fielddata/IndexFieldData.java @@ -120,11 +120,13 @@ abstract class XFieldComparatorSource extends FieldComparatorSource { protected final MultiValueMode sortMode; protected final Object missingValue; protected final Nested nested; + protected boolean enableSkipping; public XFieldComparatorSource(Object missingValue, MultiValueMode sortMode, Nested nested) { this.sortMode = sortMode; this.missingValue = missingValue; this.nested = nested; + this.enableSkipping = true; // true by default } public MultiValueMode sortMode() { @@ -135,6 +137,10 @@ public Nested nested() { return this.nested; } + public void disableSkipping() { + this.enableSkipping = false; + } + /** * Simple wrapper class around a filter that matches parent documents * and a filter that matches child documents. For every root document R, diff --git a/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java b/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java index 052a679de9765..ae8ffd8fe6b97 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java @@ -198,24 +198,35 @@ private XFieldComparatorSource comparatorSource( MultiValueMode sortMode, Nested nested ) { + final XFieldComparatorSource source; switch (targetNumericType) { case HALF_FLOAT: case FLOAT: - return new FloatValuesComparatorSource(this, missingValue, sortMode, nested); + source = new FloatValuesComparatorSource(this, missingValue, sortMode, nested); + break; case DOUBLE: - return new DoubleValuesComparatorSource(this, missingValue, sortMode, nested); + source = new DoubleValuesComparatorSource(this, missingValue, sortMode, nested); + break; case UNSIGNED_LONG: - return new UnsignedLongValuesComparatorSource(this, missingValue, sortMode, nested); + source = new UnsignedLongValuesComparatorSource(this, missingValue, sortMode, nested); + break; case DATE: - return dateComparatorSource(missingValue, sortMode, nested); + source = dateComparatorSource(missingValue, sortMode, nested); + break; case DATE_NANOSECONDS: - return dateNanosComparatorSource(missingValue, sortMode, nested); + source = dateNanosComparatorSource(missingValue, sortMode, nested); + break; case LONG: - return new LongValuesComparatorSource(this, missingValue, sortMode, nested); + source = new LongValuesComparatorSource(this, missingValue, sortMode, nested); + break; default: assert !targetNumericType.isFloatingPoint(); - return new IntValuesComparatorSource(this, missingValue, sortMode, nested); + source = new IntValuesComparatorSource(this, missingValue, sortMode, nested); } + if (targetNumericType != getNumericType()) { + source.disableSkipping(); // disable skipping logic for caste of sort field + } + return source; } protected XFieldComparatorSource dateComparatorSource(@Nullable Object missingValue, MultiValueMode sortMode, Nested nested) { diff --git a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java index f8af86c904f2c..34e86070054c9 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java +++ b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java @@ -104,7 +104,7 @@ public FieldComparator newComparator(String fieldname, int numHits, boolean 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, false) { + return new DoubleComparator(numHits, fieldname, null, reversed, enableSkipping && this.enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new DoubleLeafComparator(context) { diff --git a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java index 686bef479c179..04a34cd418520 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java +++ b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java @@ -97,7 +97,7 @@ public FieldComparator newComparator(String fieldname, int numHits, boolean 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, false) { + return new FloatComparator(numHits, fieldname, null, reversed, enableSkipping && this.enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new FloatLeafComparator(context) { diff --git a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/IntValuesComparatorSource.java b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/IntValuesComparatorSource.java index 90afa5fc64c29..d5ea1eaf7263d 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/IntValuesComparatorSource.java +++ b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/IntValuesComparatorSource.java @@ -76,7 +76,7 @@ public FieldComparator newComparator(String fieldname, int numHits, boolean e final int iMissingValue = (Integer) 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 IntComparator(numHits, null, null, reversed, false) { + return new IntComparator(numHits, fieldname, null, reversed, enableSkipping && this.enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new IntLeafComparator(context) { diff --git a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index 462092dca5110..43e033dd59716 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java @@ -120,7 +120,7 @@ public FieldComparator newComparator(String fieldname, int numHits, boolean 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, false) { + return new LongComparator(numHits, fieldname, null, reversed, enableSkipping && this.enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new LongLeafComparator(context) { diff --git a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/UnsignedLongValuesComparatorSource.java b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/UnsignedLongValuesComparatorSource.java index d8b2e9528d2cf..be56b50179114 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/UnsignedLongValuesComparatorSource.java +++ b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/UnsignedLongValuesComparatorSource.java @@ -92,7 +92,7 @@ public FieldComparator newComparator(String fieldname, int numHits, boolean e assert indexFieldData == null || fieldname.equals(indexFieldData.getFieldName()); final BigInteger ulMissingValue = (BigInteger) missingObject(missingValue, reversed); - return new UnsignedLongComparator(numHits, null, null, reversed, false) { + return new UnsignedLongComparator(numHits, fieldname, null, reversed, enableSkipping && this.enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new UnsignedLongLeafComparator(context) {