From 1ce776ae54ec99a0a601bae8a8ba392170938b92 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel Date: Tue, 31 Oct 2023 17:28:24 +0530 Subject: [PATCH 01/12] Enabling sort optimizatin back for half_float with custom comparators Signed-off-by: Chaitanya Gohel --- CHANGELOG.md | 3 +- .../test/search/260_sort_mixed.yml | 29 +++-- .../test/search/90_search_after.yml | 68 +++++++++++ .../opensearch/search/sort/FieldSortIT.java | 60 ++++++++++ .../fielddata/IndexNumericFieldData.java | 5 +- .../FloatValuesComparatorSource.java | 4 +- .../HalfFloatValuesComparatorSource.java | 55 +++++++++ .../comparators/HalfFloatComparator.java | 111 ++++++++++++++++++ 8 files changed, 324 insertions(+), 11 deletions(-) create mode 100644 server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java create mode 100644 server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 046693421d07b..f18c807db0fb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [AdmissionControl] Added changes for AdmissionControl Interceptor and AdmissionControlService for RateLimiting ([#9286](https://github.com/opensearch-project/OpenSearch/pull/9286)) - GHA to verify checklist items completion in PR descriptions ([#10800](https://github.com/opensearch-project/OpenSearch/pull/10800)) - [Remote cluster state] Restore cluster state version during remote state auto restore ([#10853](https://github.com/opensearch-project/OpenSearch/pull/10853)) +- Add back half_float BKD based sort query optimization ([#11024](https://github.com/opensearch-project/OpenSearch/pull/11024)) ### Dependencies - Bump `log4j-core` from 2.18.0 to 2.19.0 @@ -141,4 +142,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Security [Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.x...HEAD -[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.12...2.x \ No newline at end of file +[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.12...2.x diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml index ba2b18eb3b6d0..ac14f9bf7a241 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml @@ -20,6 +20,16 @@ properties: counter: type: double + + - do: + indices.create: + index: test_3 + body: + mappings: + properties: + counter: + type: half_float + - do: bulk: refresh: true @@ -33,6 +43,12 @@ - index: _index: test_2 - counter: 184.4 + - index: + _index: test_3 + - counter: 187.4 + - index: + _index: test_3 + - counter: 194.4 - do: search: @@ -40,8 +56,8 @@ rest_total_hits_as_int: true body: sort: [{ counter: desc }] - - match: { hits.total: 3 } - - length: { hits.hits: 3 } + - match: { hits.total: 5 } + - length: { hits.hits: 5 } - match: { hits.hits.0._index: test_2 } - match: { hits.hits.0._source.counter: 1223372036854775800.23 } - match: { hits.hits.0.sort.0: 1223372036854775800.23 } @@ -55,14 +71,13 @@ rest_total_hits_as_int: true body: sort: [{ counter: asc }] - - match: { hits.total: 3 } - - length: { hits.hits: 3 } + - match: { hits.total: 5 } + - length: { hits.hits: 5 } - match: { hits.hits.0._index: test_2 } - match: { hits.hits.0._source.counter: 184.4 } - match: { hits.hits.0.sort.0: 184.4 } - - match: { hits.hits.1._index: test_1 } - - match: { hits.hits.1._source.counter: 223372036854775800 } - - match: { hits.hits.1.sort.0: 223372036854775800 } + - match: { hits.hits.1._index: test_3 } + - match: { hits.hits.1._source.counter: 187.4 } --- "search across indices with mixed long, double and unsigned_long numeric types": diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml index 55e1566656faf..18ef0e49b1288 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml @@ -320,3 +320,71 @@ - length: { hits.hits: 1 } - match: { hits.hits.0._index: test } - match: { hits.hits.0._source.population: null } + +--- +"half float": + - do: + indices.create: + index: test + body: + mappings: + properties: + population: + type: half_float + - do: + bulk: + refresh: true + index: test + body: | + {"index":{}} + {"population": 184.4} + {"index":{}} + {"population": 194.4} + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + size: 1 + sort: [ { population: asc } ] + - match: { hits.total: 2 } + - length: { hits.hits: 1 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.population: 184.4 } + + # search_after with the sort + - do: + search: + index: test + rest_total_hits_as_int: true + body: + size: 1 + sort: [ { population: asc } ] + search_after: [ 184.4 ] + - match: { hits.total: 2 } + - length: { hits.hits: 1 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.population: 194.4 } + + # search_after with the sort with missing + - do: + bulk: + refresh: true + index: test + body: | + {"index":{}} + {"population": null} + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 5 + "sort": [ { "population": { "order": "asc", "missing": "_last" } } ] + search_after: [ 200 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 3 } + - length: { hits.hits: 1 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.population: null } diff --git a/server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java b/server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java index bee242b933dfd..d4980a64a3977 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java @@ -605,6 +605,9 @@ public void testSimpleSorts() throws Exception { .startObject("float_value") .field("type", "float") .endObject() + .startObject("half_float_value") + .field("type", "half_float") + .endObject() .startObject("double_value") .field("type", "double") .endObject() @@ -628,6 +631,7 @@ public void testSimpleSorts() throws Exception { .field("long_value", i) .field("unsigned_long_value", UNSIGNED_LONG_BASE.add(BigInteger.valueOf(10000 * i))) .field("float_value", 0.1 * i) + .field("half_float_value", 0.1 * i) .field("double_value", 0.1 * i) .endObject() ); @@ -794,6 +798,28 @@ public void testSimpleSorts() throws Exception { assertThat(searchResponse.toString(), not(containsString("error"))); + // HALF_FLOAT + size = 1 + random.nextInt(10); + searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("half_float_value", SortOrder.ASC).get(); + + assertHitCount(searchResponse, 10L); + assertThat(searchResponse.getHits().getHits().length, equalTo(size)); + for (int i = 0; i < size; i++) { + assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(i))); + } + + assertThat(searchResponse.toString(), not(containsString("error"))); + size = 1 + random.nextInt(10); + searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("half_float_value", SortOrder.DESC).get(); + + assertHitCount(searchResponse, 10); + assertThat(searchResponse.getHits().getHits().length, equalTo(size)); + for (int i = 0; i < size; i++) { + assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(9 - i))); + } + + assertThat(searchResponse.toString(), not(containsString("error"))); + // DOUBLE size = 1 + random.nextInt(10); searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("double_value", SortOrder.ASC).get(); @@ -1330,6 +1356,9 @@ public void testSortMVField() throws Exception { .startObject("float_values") .field("type", "float") .endObject() + .startObject("half_float_values") + .field("type", "float") + .endObject() .startObject("double_values") .field("type", "double") .endObject() @@ -1351,6 +1380,7 @@ public void testSortMVField() throws Exception { .array("short_values", 1, 5, 10, 8) .array("byte_values", 1, 5, 10, 8) .array("float_values", 1f, 5f, 10f, 8f) + .array("half_float_values", 1f, 5f, 10f, 8f) .array("double_values", 1d, 5d, 10d, 8d) .array("string_values", "01", "05", "10", "08") .endObject() @@ -1365,6 +1395,7 @@ public void testSortMVField() throws Exception { .array("short_values", 11, 15, 20, 7) .array("byte_values", 11, 15, 20, 7) .array("float_values", 11f, 15f, 20f, 7f) + .array("half_float_values", 11f, 15f, 20f, 7f) .array("double_values", 11d, 15d, 20d, 7d) .array("string_values", "11", "15", "20", "07") .endObject() @@ -1379,6 +1410,7 @@ public void testSortMVField() throws Exception { .array("short_values", 2, 1, 3, -4) .array("byte_values", 2, 1, 3, -4) .array("float_values", 2f, 1f, 3f, -4f) + .array("half_float_values", 2f, 1f, 3f, -4f) .array("double_values", 2d, 1d, 3d, -4d) .array("string_values", "02", "01", "03", "!4") .endObject() @@ -1585,6 +1617,34 @@ public void testSortMVField() throws Exception { assertThat(searchResponse.getHits().getAt(2).getId(), equalTo(Integer.toString(3))); assertThat(((Number) searchResponse.getHits().getAt(2).getSortValues()[0]).floatValue(), equalTo(3f)); + searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(10).addSort("half_float_values", SortOrder.ASC).get(); + + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(3L)); + assertThat(searchResponse.getHits().getHits().length, equalTo(3)); + + assertThat(searchResponse.getHits().getAt(0).getId(), equalTo(Integer.toString(3))); + assertThat(((Number) searchResponse.getHits().getAt(0).getSortValues()[0]).floatValue(), equalTo(-4f)); + + assertThat(searchResponse.getHits().getAt(1).getId(), equalTo(Integer.toString(1))); + assertThat(((Number) searchResponse.getHits().getAt(1).getSortValues()[0]).floatValue(), equalTo(1f)); + + assertThat(searchResponse.getHits().getAt(2).getId(), equalTo(Integer.toString(2))); + assertThat(((Number) searchResponse.getHits().getAt(2).getSortValues()[0]).floatValue(), equalTo(7f)); + + searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(10).addSort("half_float_values", SortOrder.DESC).get(); + + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(3L)); + assertThat(searchResponse.getHits().getHits().length, equalTo(3)); + + assertThat(searchResponse.getHits().getAt(0).getId(), equalTo(Integer.toString(2))); + assertThat(((Number) searchResponse.getHits().getAt(0).getSortValues()[0]).floatValue(), equalTo(20f)); + + assertThat(searchResponse.getHits().getAt(1).getId(), equalTo(Integer.toString(1))); + assertThat(((Number) searchResponse.getHits().getAt(1).getSortValues()[0]).floatValue(), equalTo(10f)); + + assertThat(searchResponse.getHits().getAt(2).getId(), equalTo(Integer.toString(3))); + assertThat(((Number) searchResponse.getHits().getAt(2).getSortValues()[0]).floatValue(), equalTo(3f)); + searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(10).addSort("double_values", SortOrder.ASC).get(); assertThat(searchResponse.getHits().getTotalHits().value, equalTo(3L)); 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 6fc074fe0de95..b0ff944d014de 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java @@ -42,6 +42,7 @@ import org.opensearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested; import org.opensearch.index.fielddata.fieldcomparator.DoubleValuesComparatorSource; import org.opensearch.index.fielddata.fieldcomparator.FloatValuesComparatorSource; +import org.opensearch.index.fielddata.fieldcomparator.HalfFloatValuesComparatorSource; import org.opensearch.index.fielddata.fieldcomparator.IntValuesComparatorSource; import org.opensearch.index.fielddata.fieldcomparator.LongValuesComparatorSource; import org.opensearch.index.fielddata.fieldcomparator.UnsignedLongValuesComparatorSource; @@ -220,6 +221,8 @@ private XFieldComparatorSource comparatorSource( final XFieldComparatorSource source; switch (targetNumericType) { case HALF_FLOAT: + source = new HalfFloatValuesComparatorSource(this, missingValue, sortMode, nested); + break; case FLOAT: source = new FloatValuesComparatorSource(this, missingValue, sortMode, nested); break; @@ -242,7 +245,7 @@ private XFieldComparatorSource comparatorSource( assert !targetNumericType.isFloatingPoint(); source = new IntValuesComparatorSource(this, missingValue, sortMode, nested); } - if (targetNumericType != getNumericType() || getNumericType() == NumericType.HALF_FLOAT) { + if (targetNumericType != getNumericType()) { source.disableSkipping(); // disable skipping logic for cast of sort field } return source; 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 04a34cd418520..abf872dd235e5 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 @@ -61,7 +61,7 @@ */ public class FloatValuesComparatorSource extends IndexFieldData.XFieldComparatorSource { - private final IndexNumericFieldData indexFieldData; + protected final IndexNumericFieldData indexFieldData; public FloatValuesComparatorSource( IndexNumericFieldData indexFieldData, @@ -78,7 +78,7 @@ public SortField.Type reducedType() { return SortField.Type.FLOAT; } - private NumericDoubleValues getNumericDocValues(LeafReaderContext context, float missingValue) throws IOException { + protected NumericDoubleValues getNumericDocValues(LeafReaderContext context, float missingValue) throws IOException { final SortedNumericDoubleValues values = indexFieldData.load(context).getDoubleValues(); if (nested == null) { return FieldData.replaceMissing(sortMode.select(values), missingValue); diff --git a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java new file mode 100644 index 0000000000000..f63c4110bbdda --- /dev/null +++ b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java @@ -0,0 +1,55 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.fielddata.fieldcomparator; + +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.search.FieldComparator; +import org.apache.lucene.search.LeafFieldComparator; +import org.opensearch.index.fielddata.IndexNumericFieldData; +import org.opensearch.index.search.comparators.HalfFloatComparator; +import org.opensearch.search.MultiValueMode; + +import java.io.IOException; + +/** + * Comparator source for half_float values. + * + * @opensearch.internal + */ +public class HalfFloatValuesComparatorSource extends FloatValuesComparatorSource { + public HalfFloatValuesComparatorSource( + IndexNumericFieldData indexFieldData, + Object missingValue, + MultiValueMode sortMode, + Nested nested + ) { + super(indexFieldData, missingValue, sortMode, nested); + } + + @Override + public FieldComparator newComparator(String fieldname, int numHits, boolean enableSkipping, boolean reversed) { + assert indexFieldData == null || fieldname.equals(indexFieldData.getFieldName()); + + 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 && this.enableSkipping) { + @Override + public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { + return new HalfFloatLeafComparator(context) { + @Override + protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) throws IOException { + return HalfFloatValuesComparatorSource.this.getNumericDocValues(context, fMissingValue).getRawFloatValues(); + } + }; + } + }; + } +} diff --git a/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java b/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java new file mode 100644 index 0000000000000..685c0bb9bbf3d --- /dev/null +++ b/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java @@ -0,0 +1,111 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.search.comparators; + +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.sandbox.document.HalfFloatPoint; +import org.apache.lucene.search.LeafFieldComparator; +import org.apache.lucene.search.comparators.NumericComparator; + +import java.io.IOException; + +/** + * The comparator for unsigned long numeric type. + * Comparator based on {@link Float#compare} for {@code numHits}. This comparator provides a + * skipping functionality – an iterator that can skip over non-competitive documents. + */ +public class HalfFloatComparator extends NumericComparator { + private final float[] values; + protected float topValue; + protected float bottom; + + public HalfFloatComparator(int numHits, String field, Float missingValue, boolean reverse, boolean enableSkipping) { + super(field, missingValue != null ? missingValue : 0.0f, reverse, enableSkipping, HalfFloatPoint.BYTES); + values = new float[numHits]; + } + + @Override + public int compare(int slot1, int slot2) { + return Float.compare(values[slot1], values[slot2]); + } + + @Override + public void setTopValue(Float value) { + super.setTopValue(value); + topValue = value; + } + + @Override + public Float value(int slot) { + return Float.valueOf(values[slot]); + } + + @Override + public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { + return new HalfFloatLeafComparator(context); + } + + /** Leaf comparator for {@link HalfFloatComparator} that provides skipping functionality */ + public class HalfFloatLeafComparator extends NumericLeafComparator { + + public HalfFloatLeafComparator(LeafReaderContext context) throws IOException { + super(context); + } + + private float getValueForDoc(int doc) throws IOException { + if (docValues.advanceExact(doc)) { + return Float.intBitsToFloat((int) 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 Float.compare(bottom, getValueForDoc(doc)); + } + + @Override + public int compareTop(int doc) throws IOException { + return Float.compare(topValue, getValueForDoc(doc)); + } + + @Override + public void copy(int slot, int doc) throws IOException { + values[slot] = getValueForDoc(doc); + super.copy(slot, doc); + } + + @Override + protected int compareMissingValueWithBottomValue() { + return Float.compare(missingValue, bottom); + } + + @Override + protected int compareMissingValueWithTopValue() { + return Float.compare(missingValue, topValue); + } + + @Override + protected void encodeBottom(byte[] packedValue) { + HalfFloatPoint.encodeDimension(bottom, packedValue, 0); + } + + @Override + protected void encodeTop(byte[] packedValue) { + HalfFloatPoint.encodeDimension(topValue, packedValue, 0); + } + } +} From 9cce0f639ed728a102d243db3dbef3c04067c4c3 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel Date: Tue, 31 Oct 2023 19:19:03 +0530 Subject: [PATCH 02/12] Fixing tests Signed-off-by: Chaitanya Gohel --- .../test/search/260_sort_mixed.yml | 110 ++++++++++++++---- .../test/search/90_search_after.yml | 4 + .../comparators/HalfFloatComparator.java | 2 +- 3 files changed, 94 insertions(+), 22 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml index ac14f9bf7a241..d291093970c7e 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml @@ -21,15 +21,6 @@ counter: type: double - - do: - indices.create: - index: test_3 - body: - mappings: - properties: - counter: - type: half_float - - do: bulk: refresh: true @@ -43,12 +34,6 @@ - index: _index: test_2 - counter: 184.4 - - index: - _index: test_3 - - counter: 187.4 - - index: - _index: test_3 - - counter: 194.4 - do: search: @@ -56,8 +41,8 @@ rest_total_hits_as_int: true body: sort: [{ counter: desc }] - - match: { hits.total: 5 } - - length: { hits.hits: 5 } + - match: { hits.total: 3 } + - length: { hits.hits: 3 } - match: { hits.hits.0._index: test_2 } - match: { hits.hits.0._source.counter: 1223372036854775800.23 } - match: { hits.hits.0.sort.0: 1223372036854775800.23 } @@ -71,13 +56,14 @@ rest_total_hits_as_int: true body: sort: [{ counter: asc }] - - match: { hits.total: 5 } - - length: { hits.hits: 5 } + - match: { hits.total: 3 } + - length: { hits.hits: 3 } - match: { hits.hits.0._index: test_2 } - match: { hits.hits.0._source.counter: 184.4 } - match: { hits.hits.0.sort.0: 184.4 } - - match: { hits.hits.1._index: test_3 } - - match: { hits.hits.1._source.counter: 187.4 } + - match: { hits.hits.1._index: test_1 } + - match: { hits.hits.1._source.counter: 223372036854775800 } + - match: { hits.hits.1.sort.0: 223372036854775800 } --- "search across indices with mixed long, double and unsigned_long numeric types": @@ -134,3 +120,85 @@ - match: { status: 400 } - match: { error.type: search_phase_execution_exception } - match: { error.caused_by.reason: "Can't do sort across indices, as a field has [unsigned_long] type in one index, and different type in another index!" } + +--- +"search across indices with mixed long and double and float numeric types": + - skip: + version: " - 2.10.99" + reason: half float was broken before 2.11 + + - do: + indices.create: + index: test_1 + body: + mappings: + properties: + counter: + type: long + + - do: + indices.create: + index: test_2 + body: + mappings: + properties: + counter: + type: double + + - do: + indices.create: + index: test_3 + body: + mappings: + properties: + counter: + type: half_float + + - do: + bulk: + refresh: true + body: + - index: + _index: test_1 + - counter: 223372036854775800 + - index: + _index: test_2 + - counter: 1223372036854775800.23 + - index: + _index: test_2 + - counter: 184.4 + - index: + _index: test_3 + - counter: 187.4 + - index: + _index: test_3 + - counter: 194.4 + + - do: + search: + index: test_* + rest_total_hits_as_int: true + body: + sort: [{ counter: desc }] + - match: { hits.total: 5 } + - length: { hits.hits: 5 } + - match: { hits.hits.0._index: test_2 } + - match: { hits.hits.0._source.counter: 1223372036854775800.23 } + - match: { hits.hits.0.sort.0: 1223372036854775800.23 } + - match: { hits.hits.1._index: test_1 } + - match: { hits.hits.1._source.counter: 223372036854775800 } + - match: { hits.hits.1.sort.0: 223372036854775800 } + + - do: + search: + index: test_* + rest_total_hits_as_int: true + body: + sort: [{ counter: asc }] + - match: { hits.total: 5 } + - length: { hits.hits: 5 } + - match: { hits.hits.0._index: test_2 } + - match: { hits.hits.0._source.counter: 184.4 } + - match: { hits.hits.0.sort.0: 184.4 } + - match: { hits.hits.1._index: test_3 } + - match: { hits.hits.1._source.counter: 187.4 } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml index 18ef0e49b1288..2a70eef0cbf73 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml @@ -323,6 +323,10 @@ --- "half float": + - skip: + version: " - 2.10.99" + reason: half_float was broken for 2.10 and earlier + - do: indices.create: index: test diff --git a/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java b/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java index 685c0bb9bbf3d..732fc8399059f 100644 --- a/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java +++ b/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java @@ -16,7 +16,7 @@ import java.io.IOException; /** - * The comparator for unsigned long numeric type. + * The comparator for unsigned half_float numeric type. * Comparator based on {@link Float#compare} for {@code numHits}. This comparator provides a * skipping functionality – an iterator that can skip over non-competitive documents. */ From 4eefa778987f563d69397a4444f2953309fef3a4 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel Date: Tue, 31 Oct 2023 20:41:24 +0530 Subject: [PATCH 03/12] Adding test for Indecx sort half_float Signed-off-by: Chaitanya Gohel --- .../test/indices.sort/10_basic.yml | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.sort/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.sort/10_basic.yml index b9089689b0cf1..3b7ea15164e9f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.sort/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.sort/10_basic.yml @@ -156,3 +156,23 @@ query: {"range": { "rank": { "from": 0 } } } track_total_hits: false size: 3 + +--- +"Index Sort half float": + - do: + catch: bad_request + indices.create: + index: test + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + index.sort.field: rank + mappings: + properties: + rank: + type: half_float + + # This should failed with 400 as half_float is not supported for index sort + - match: { status: 400 } + - match: { error.type: illegal_argument_exception } From 41cba16fba072e2101cd4a1041449c1560918449 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel Date: Tue, 31 Oct 2023 23:24:09 +0530 Subject: [PATCH 04/12] Making indexFieldData provate in FloatValuesComparatorSource Signed-off-by: Chaitanya Gohel --- .../FloatValuesComparatorSource.java | 4 ++-- .../HalfFloatValuesComparatorSource.java | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) 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 abf872dd235e5..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 @@ -61,7 +61,7 @@ */ public class FloatValuesComparatorSource extends IndexFieldData.XFieldComparatorSource { - protected final IndexNumericFieldData indexFieldData; + private final IndexNumericFieldData indexFieldData; public FloatValuesComparatorSource( IndexNumericFieldData indexFieldData, @@ -78,7 +78,7 @@ public SortField.Type reducedType() { return SortField.Type.FLOAT; } - protected NumericDoubleValues getNumericDocValues(LeafReaderContext context, float missingValue) throws IOException { + private NumericDoubleValues getNumericDocValues(LeafReaderContext context, float missingValue) throws IOException { final SortedNumericDoubleValues values = indexFieldData.load(context).getDoubleValues(); if (nested == null) { return FieldData.replaceMissing(sortMode.select(values), missingValue); diff --git a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java index f63c4110bbdda..7e3936be1d8a5 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java +++ b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java @@ -10,9 +10,14 @@ import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.search.DocIdSetIterator; import org.apache.lucene.search.FieldComparator; import org.apache.lucene.search.LeafFieldComparator; +import org.apache.lucene.util.BitSet; +import org.opensearch.index.fielddata.FieldData; import org.opensearch.index.fielddata.IndexNumericFieldData; +import org.opensearch.index.fielddata.NumericDoubleValues; +import org.opensearch.index.fielddata.SortedNumericDoubleValues; import org.opensearch.index.search.comparators.HalfFloatComparator; import org.opensearch.search.MultiValueMode; @@ -24,6 +29,8 @@ * @opensearch.internal */ public class HalfFloatValuesComparatorSource extends FloatValuesComparatorSource { + private final IndexNumericFieldData indexFieldData; + public HalfFloatValuesComparatorSource( IndexNumericFieldData indexFieldData, Object missingValue, @@ -31,6 +38,7 @@ public HalfFloatValuesComparatorSource( Nested nested ) { super(indexFieldData, missingValue, sortMode, nested); + this.indexFieldData = indexFieldData; } @Override @@ -52,4 +60,16 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String } }; } + + private NumericDoubleValues getNumericDocValues(LeafReaderContext context, float missingValue) throws IOException { + final SortedNumericDoubleValues values = indexFieldData.load(context).getDoubleValues(); + if (nested == null) { + return FieldData.replaceMissing(sortMode.select(values), missingValue); + } else { + final BitSet rootDocs = nested.rootDocs(context); + final DocIdSetIterator innerDocs = nested.innerDocs(context); + final int maxChildren = nested.getNestedSort() != null ? nested.getNestedSort().getMaxChildren() : Integer.MAX_VALUE; + return sortMode.select(values, missingValue, rootDocs, innerDocs, context.reader().maxDoc(), maxChildren); + } + } } From 29bdd915a020eba72caa5e97889072c48b48abed Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel <104654647+gashutos@users.noreply.github.com> Date: Thu, 2 Nov 2023 11:20:15 +0530 Subject: [PATCH 05/12] Update server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java Co-authored-by: Andriy Redko Signed-off-by: Chaitanya Gohel <104654647+gashutos@users.noreply.github.com> --- .../index/search/comparators/HalfFloatComparator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java b/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java index 732fc8399059f..6244fa647b042 100644 --- a/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java +++ b/server/src/main/java/org/opensearch/index/search/comparators/HalfFloatComparator.java @@ -16,7 +16,7 @@ import java.io.IOException; /** - * The comparator for unsigned half_float numeric type. + * The comparator for half_float numeric type. * Comparator based on {@link Float#compare} for {@code numHits}. This comparator provides a * skipping functionality – an iterator that can skip over non-competitive documents. */ From d4715bdd934df423d50c52b107210b28a5730812 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel Date: Mon, 6 Nov 2023 01:25:00 +0530 Subject: [PATCH 06/12] Adding missing value instead null Signed-off-by: Chaitanya Gohel --- .../test/search/260_sort_mixed.yml | 2 + .../test/search/90_search_after.yml | 55 ++++++++++++++++--- .../DoubleValuesComparatorSource.java | 2 +- .../FloatValuesComparatorSource.java | 2 +- .../HalfFloatValuesComparatorSource.java | 2 +- .../IntValuesComparatorSource.java | 2 +- .../LongValuesComparatorSource.java | 2 +- .../UnsignedLongValuesComparatorSource.java | 2 +- 8 files changed, 55 insertions(+), 14 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml index d291093970c7e..a04dc308b2a06 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_mixed.yml @@ -188,6 +188,8 @@ - match: { hits.hits.1._index: test_1 } - match: { hits.hits.1._source.counter: 223372036854775800 } - match: { hits.hits.1.sort.0: 223372036854775800 } + - match: { hits.hits.2._index: test_3 } + - match: { hits.hits.2._source.counter: 194.4 } - do: search: diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml index 2a70eef0cbf73..31db42acafaac 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml @@ -344,20 +344,30 @@ {"population": 184.4} {"index":{}} {"population": 194.4} + {"index":{}} + {"population": 144.4} + {"index":{}} + {"population": 174.4} + {"index":{}} + {"population": 164.4} - do: search: index: test rest_total_hits_as_int: true body: - size: 1 + size: 3 sort: [ { population: asc } ] - - match: { hits.total: 2 } - - length: { hits.hits: 1 } + - match: { hits.total: 5 } + - length: { hits.hits: 3 } - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.population: 184.4 } + - match: { hits.hits.0._source.population: 144.4 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.population: 164.4 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.population: 174.4 } - # search_after with the sort + # search_after with the asc sort - do: search: index: test @@ -366,12 +376,26 @@ size: 1 sort: [ { population: asc } ] search_after: [ 184.4 ] - - match: { hits.total: 2 } + - match: { hits.total: 5 } - length: { hits.hits: 1 } - match: { hits.hits.0._index: test } - match: { hits.hits.0._source.population: 194.4 } - # search_after with the sort with missing + # search_after with the desc sort + - do: + search: + index: test + rest_total_hits_as_int: true + body: + size: 1 + sort: [ { population: desc } ] + search_after: [ 164.4 ] + - match: { hits.total: 5 } + - length: { hits.hits: 1 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.population: 164.4 } + + # search_after with the asc sort with missing - do: bulk: refresh: true @@ -388,7 +412,22 @@ "sort": [ { "population": { "order": "asc", "missing": "_last" } } ] search_after: [ 200 ] # making it out of min/max so only missing value hit is qualified - - match: { hits.total: 3 } + - match: { hits.total: 6 } + - length: { hits.hits: 1 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.population: null } + + # search_after with the desc sort with missing + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 5 + "sort": [ { "population": { "order": "desc", "missing": "_last" } } ] + search_after: [ 100 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 6 } - length: { hits.hits: 1 } - match: { hits.hits.0._index: test } - match: { hits.hits.0._source.population: null } 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 e70916c33882c..eb6fd2216d9b5 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, fieldname, null, reversed, enableSkipping && this.enableSkipping) { + return new DoubleComparator(numHits, fieldname, dMissingValue, 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 04a34cd418520..e76e6ea1c6f08 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, fieldname, null, reversed, enableSkipping && this.enableSkipping) { + return new FloatComparator(numHits, fieldname, fMissingValue, 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/HalfFloatValuesComparatorSource.java b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java index 7e3936be1d8a5..931d3423536b8 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java +++ b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java @@ -48,7 +48,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 HalfFloatComparator(numHits, fieldname, null, reversed, enableSkipping && this.enableSkipping) { + return new HalfFloatComparator(numHits, fieldname, fMissingValue, reversed, enableSkipping && this.enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new HalfFloatLeafComparator(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 d5ea1eaf7263d..9dd95f79580cd 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, fieldname, null, reversed, enableSkipping && this.enableSkipping) { + return new IntComparator(numHits, fieldname, iMissingValue, 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 2c67d7ab7e86a..60e978c4c1452 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, fieldname, null, reversed, enableSkipping && this.enableSkipping) { + return new LongComparator(numHits, fieldname, lMissingValue, 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 5416d4fe0fb91..9d1edde1fe08f 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, fieldname, null, reversed, enableSkipping && this.enableSkipping) { + return new UnsignedLongComparator(numHits, fieldname, ulMissingValue, reversed, enableSkipping && this.enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new UnsignedLongLeafComparator(context) { From 99087149b257d163565eb709692d20484cc11127 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel Date: Tue, 7 Nov 2023 12:06:45 +0530 Subject: [PATCH 07/12] Adding more tests for desc order sort Signed-off-by: Chaitanya Gohel --- .../test/search/90_search_after.yml | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml index 31db42acafaac..68ac925fcaebe 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml @@ -351,6 +351,22 @@ {"index":{}} {"population": 164.4} + - do: + search: + index: test + rest_total_hits_as_int: true + body: + size: 3 + sort: [ { population: desc } ] + - match: { hits.total: 5 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.population: 194.4 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.population: 184.4 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.population: 174.4 } + - do: search: index: test @@ -375,7 +391,7 @@ body: size: 1 sort: [ { population: asc } ] - search_after: [ 184.4 ] + search_after: [ 184.375 ] # this is ruonded sort value in sort result - match: { hits.total: 5 } - length: { hits.hits: 1 } - match: { hits.hits.0._index: test } @@ -389,11 +405,11 @@ body: size: 1 sort: [ { population: desc } ] - search_after: [ 164.4 ] + search_after: [ 164.375 ] # this is rounded sort value in sort result - match: { hits.total: 5 } - length: { hits.hits: 1 } - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.population: 164.4 } + - match: { hits.hits.0._source.population: 144.4 } # search_after with the asc sort with missing - do: From 647c3764f600c2ae6577f0871ccc13e45754eec8 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel <104654647+gashutos@users.noreply.github.com> Date: Tue, 7 Nov 2023 18:03:30 +0530 Subject: [PATCH 08/12] Update rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml Co-authored-by: Prabhakar Sithanandam Signed-off-by: Chaitanya Gohel <104654647+gashutos@users.noreply.github.com> --- .../resources/rest-api-spec/test/search/90_search_after.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml index 68ac925fcaebe..1563daba9de6d 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml @@ -391,7 +391,7 @@ body: size: 1 sort: [ { population: asc } ] - search_after: [ 184.375 ] # this is ruonded sort value in sort result + search_after: [ 184.375 ] # this is rounded sort value in sort result - match: { hits.total: 5 } - length: { hits.hits: 1 } - match: { hits.hits.0._index: test } From a2015ef1c630b169a711e5487312acd3363f608a Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel Date: Tue, 7 Nov 2023 20:18:58 +0530 Subject: [PATCH 09/12] Adding tests in case missing values are competitive Signed-off-by: Chaitanya Gohel --- .../test/search/90_search_after.yml | 296 ++++++++++++++++++ 1 file changed, 296 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml index 1563daba9de6d..cdf1dc6336577 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml @@ -447,3 +447,299 @@ - length: { hits.hits: 1 } - match: { hits.hits.0._index: test } - match: { hits.hits.0._source.population: null } + +--- +"numeric skipping logic with competitive missing value": + - skip: + version: " - 2.10.99" + reason: newly added test, half_float was broken for 2.10 and earlier so keeping this only + +# This test checks if skipping logic is skipped in case missing values are competitive +# for all numeric type int, long, float, half_float, double, unsigned_long. +# We are inserting 24 documents with some missing values and giving search after parameter +# as missing value. The secondary sort field is on id which doesn't have missing value. +# In case skipping logic is applied in Lucene, it will skipp all documents with primary sort field +# missing value even though it should list sort by secondary field id with missing value primary field. +# This test is addressing bugs like here https://github.com/opensearch-project/OpenSearch/issues/9537 + + - do: + indices.create: + index: test + body: + mappings: + properties: + halffloat: + type: half_float + long: + type: long + int: + type: integer + float: + type: float + double: + type: double + unsignedlong: + type: unsigned_long + - do: + bulk: + refresh: true + index: test + body: | + {"index":{}} + {"id": 1, "halffloat": 1, "long": 1, "int": 1, "float": 1, "double": 1, "unsignedlong": 1} + {"index":{}} + {"id": 2, "halffloat": 2, "long": 2, "int": 2, "float": 2, "double": 2, "unsignedlong": 2} + {"index":{}} + {"id": 3, "halffloat": 3, "long": 3, "int": 3, "float": 3, "double": 3, "unsignedlong": 3} + {"index":{}} + {"id": 4, "halffloat": 4, "long": 4, "int": 4, "float": 4, "double": 4, "unsignedlong": 4} + {"index":{}} + {"id": 5, "halffloat": 5, "long": 5, "int": 5, "float": 5, "double": 5, "unsignedlong": 5} + {"index":{}} + {"id": 6, "halffloat": 6, "long": 6, "int": 6, "float": 6, "double": 6, "unsignedlong": 6} + {"index":{}} + {"id": 7, "halffloat": 7, "long": 7, "int": 7, "float": 7, "double": 7, "unsignedlong": 7} + {"index":{}} + {"id": 8, "halffloat": 8, "long": 8, "int": 8, "float": 8, "double": 8, "unsignedlong": 8} + {"index":{}} + {"id": 9, "halffloat": 9, "long": 9, "int": 9, "float": 9, "double": 9, "unsignedlong": 9} + {"index":{}} + {"id": 10, "halffloat": 10, "long": 10, "int": 10, "float": 10, "double": 10, "unsignedlong": 10} + {"index":{}} + {"id": 11, "halffloat": 11, "long": 11, "int": 11, "float": 11, "double": 11, "unsignedlong": 11} + {"index":{}} + {"id": 12, "halffloat": 12, "long": 12, "int": 12, "float": 12, "double": 12, "unsignedlong": 12} + {"index":{}} + {"id": 13, "halffloat": 13, "long": 13, "int": 13, "float": 13, "double": 13, "unsignedlong": 13} + {"index":{}} + {"id": 14, "halffloat": 14, "long": 14, "int": 14, "float": 14, "double": 14, "unsignedlong": 14} + {"index":{}} + {"id": 15, "halffloat": 15, "long": 15, "int": 15, "float": 15, "double": 15, "unsignedlong": 15} + {"index":{}} + {"id": 16, "halffloat": 16, "long": 16, "int": 16, "float": 16, "double": 16, "unsignedlong": 16} + {"index":{}} + {"id": 17, "halffloat": 17, "long": 17, "int": 17, "float": 17, "double": 17, "unsignedlong": 17} + {"index":{}} + {"id": 18, "halffloat": 18, "long": 18, "int": 18, "float": 18, "double": 18, "unsignedlong": 18} + {"index":{}} + {"id": 19, "halffloat": 19, "long": 19, "int": 19, "float": 19, "double": 19, "unsignedlong": 19} + {"index":{}} + {"id": 20, "halffloat": 20, "long": 20, "int": 20, "float": 20, "double": 20, "unsignedlong": 20} + {"index":{}} + {"id": 21, "halffloat": null, "long": null, "int": null, "float": null, "double": null, "unsignedlong": null} + {"index":{}} + {"id": 22, "halffloat": null, "long": null, "int": null, "float": null, "double": null, "unsignedlong": null} + {"index":{}} + {"id": 23, "halffloat": null, "long": null, "int": null, "float": null, "double": null, "unsignedlong": null} + {"index":{}} + {"id": 24, "halffloat": null, "long": null, "int": null, "float": null, "double": null, "unsignedlong": null} + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "halffloat": { "order": "asc" } }, { "id": { "order": "asc" } } ] + search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.halffloat: null } + - match: { hits.hits.0._source.id: 21 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.halffloat: null } + - match: { hits.hits.1._source.id: 22 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.halffloat: null } + - match: { hits.hits.2._source.id: 23 } + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "halffloat": { "order": "desc" } }, { "id": { "order": "desc" } } ] + search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.halffloat: null } + - match: { hits.hits.0._source.id: 24 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.halffloat: null } + - match: { hits.hits.1._source.id: 23 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.halffloat: null } + - match: { hits.hits.2._source.id: 22 } + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "long": { "order": "asc" } }, { "id": { "order": "asc" } } ] + search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.long: null } + - match: { hits.hits.0._source.id: 21 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.long: null } + - match: { hits.hits.1._source.id: 22 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.long: null } + - match: { hits.hits.2._source.id: 23 } + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "long": { "order": "desc" } }, { "id": { "order": "desc" } } ] + search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.long: null } + - match: { hits.hits.0._source.id: 24 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.long: null } + - match: { hits.hits.1._source.id: 23 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.long: null } + - match: { hits.hits.2._source.id: 22 } + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "int": { "order": "asc" } }, { "id": { "order": "asc" } } ] + search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.int: null } + - match: { hits.hits.0._source.id: 21 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.int: null } + - match: { hits.hits.1._source.id: 22 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.int: null } + - match: { hits.hits.2._source.id: 23 } + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "int": { "order": "desc" } }, { "id": { "order": "desc" } } ] + search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.int: null } + - match: { hits.hits.0._source.id: 24 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.int: null } + - match: { hits.hits.1._source.id: 23 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.int: null } + - match: { hits.hits.2._source.id: 22 } + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "double": { "order": "asc" } }, { "id": { "order": "asc" } } ] + search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.double: null } + - match: { hits.hits.0._source.id: 21 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.double: null } + - match: { hits.hits.1._source.id: 22 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.double: null } + - match: { hits.hits.2._source.id: 23 } + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "double": { "order": "desc" } }, { "id": { "order": "desc" } } ] + search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.double: null } + - match: { hits.hits.0._source.id: 24 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.double: null } + - match: { hits.hits.1._source.id: 23 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.double: null } + - match: { hits.hits.2._source.id: 22 } + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "unsignedlong": { "order": "asc" } }, { "id": { "order": "asc" } } ] + search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.unsignedlong: null } + - match: { hits.hits.0._source.id: 21 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.unsignedlong: null } + - match: { hits.hits.1._source.id: 22 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.unsignedlong: null } + - match: { hits.hits.2._source.id: 23 } + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "unsignedlong": { "order": "desc" } }, { "id": { "order": "desc" } } ] + search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.unsignedlong: null } + - match: { hits.hits.0._source.id: 24 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.unsignedlong: null } + - match: { hits.hits.1._source.id: 23 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.unsignedlong: null } + - match: { hits.hits.2._source.id: 22 } From 563ccb98f6572723c5d378074ac870beab00d108 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel Date: Tue, 7 Nov 2023 22:21:59 +0530 Subject: [PATCH 10/12] chanheing newly added test supported version 3.0.0 Signed-off-by: Chaitanya Gohel --- .../resources/rest-api-spec/test/search/90_search_after.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml index cdf1dc6336577..f4ca441ff427d 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml @@ -451,8 +451,8 @@ --- "numeric skipping logic with competitive missing value": - skip: - version: " - 2.10.99" - reason: newly added test, half_float was broken for 2.10 and earlier so keeping this only + version: " - 2.12.99" + reason: newly added test, supported from 3.0.0 # This test checks if skipping logic is skipped in case missing values are competitive # for all numeric type int, long, float, half_float, double, unsigned_long. From e3a5094607a953a922be4399118ea0df3047c043 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel Date: Tue, 7 Nov 2023 22:56:50 +0530 Subject: [PATCH 11/12] Assing missing float tests Signed-off-by: Chaitanya Gohel --- .../test/search/90_search_after.yml | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml index f4ca441ff427d..9f6a9c6871ed6 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml @@ -660,6 +660,48 @@ - match: { hits.hits.2._source.int: null } - match: { hits.hits.2._source.id: 22 } + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "float": { "order": "asc" } }, { "id": { "order": "asc" } } ] + search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.float: null } + - match: { hits.hits.0._source.id: 21 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.float: null } + - match: { hits.hits.1._source.id: 22 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.float: null } + - match: { hits.hits.2._source.id: 23 } + + - do: + search: + index: test + rest_total_hits_as_int: true + body: + "size": 3 + "sort": [ { "float": { "order": "desc" } }, { "id": { "order": "desc" } } ] + search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified + + - match: { hits.total: 24 } + - length: { hits.hits: 3 } + - match: { hits.hits.0._index: test } + - match: { hits.hits.0._source.float: null } + - match: { hits.hits.0._source.id: 24 } + - match: { hits.hits.1._index: test } + - match: { hits.hits.1._source.float: null } + - match: { hits.hits.1._source.id: 23 } + - match: { hits.hits.2._index: test } + - match: { hits.hits.2._source.float: null } + - match: { hits.hits.2._source.id: 22 } + - do: search: index: test From 2a4a43009bab047d59aeff65e4166c57bf1796e9 Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel Date: Wed, 8 Nov 2023 20:51:47 +0530 Subject: [PATCH 12/12] Remove missing value change to be part of another PR Signed-off-by: Chaitanya Gohel --- .../test/search/90_search_after.yml | 338 ------------------ .../DoubleValuesComparatorSource.java | 2 +- .../FloatValuesComparatorSource.java | 2 +- .../HalfFloatValuesComparatorSource.java | 2 +- .../IntValuesComparatorSource.java | 2 +- .../LongValuesComparatorSource.java | 2 +- .../UnsignedLongValuesComparatorSource.java | 2 +- 7 files changed, 6 insertions(+), 344 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml index 9f6a9c6871ed6..1563daba9de6d 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml @@ -447,341 +447,3 @@ - length: { hits.hits: 1 } - match: { hits.hits.0._index: test } - match: { hits.hits.0._source.population: null } - ---- -"numeric skipping logic with competitive missing value": - - skip: - version: " - 2.12.99" - reason: newly added test, supported from 3.0.0 - -# This test checks if skipping logic is skipped in case missing values are competitive -# for all numeric type int, long, float, half_float, double, unsigned_long. -# We are inserting 24 documents with some missing values and giving search after parameter -# as missing value. The secondary sort field is on id which doesn't have missing value. -# In case skipping logic is applied in Lucene, it will skipp all documents with primary sort field -# missing value even though it should list sort by secondary field id with missing value primary field. -# This test is addressing bugs like here https://github.com/opensearch-project/OpenSearch/issues/9537 - - - do: - indices.create: - index: test - body: - mappings: - properties: - halffloat: - type: half_float - long: - type: long - int: - type: integer - float: - type: float - double: - type: double - unsignedlong: - type: unsigned_long - - do: - bulk: - refresh: true - index: test - body: | - {"index":{}} - {"id": 1, "halffloat": 1, "long": 1, "int": 1, "float": 1, "double": 1, "unsignedlong": 1} - {"index":{}} - {"id": 2, "halffloat": 2, "long": 2, "int": 2, "float": 2, "double": 2, "unsignedlong": 2} - {"index":{}} - {"id": 3, "halffloat": 3, "long": 3, "int": 3, "float": 3, "double": 3, "unsignedlong": 3} - {"index":{}} - {"id": 4, "halffloat": 4, "long": 4, "int": 4, "float": 4, "double": 4, "unsignedlong": 4} - {"index":{}} - {"id": 5, "halffloat": 5, "long": 5, "int": 5, "float": 5, "double": 5, "unsignedlong": 5} - {"index":{}} - {"id": 6, "halffloat": 6, "long": 6, "int": 6, "float": 6, "double": 6, "unsignedlong": 6} - {"index":{}} - {"id": 7, "halffloat": 7, "long": 7, "int": 7, "float": 7, "double": 7, "unsignedlong": 7} - {"index":{}} - {"id": 8, "halffloat": 8, "long": 8, "int": 8, "float": 8, "double": 8, "unsignedlong": 8} - {"index":{}} - {"id": 9, "halffloat": 9, "long": 9, "int": 9, "float": 9, "double": 9, "unsignedlong": 9} - {"index":{}} - {"id": 10, "halffloat": 10, "long": 10, "int": 10, "float": 10, "double": 10, "unsignedlong": 10} - {"index":{}} - {"id": 11, "halffloat": 11, "long": 11, "int": 11, "float": 11, "double": 11, "unsignedlong": 11} - {"index":{}} - {"id": 12, "halffloat": 12, "long": 12, "int": 12, "float": 12, "double": 12, "unsignedlong": 12} - {"index":{}} - {"id": 13, "halffloat": 13, "long": 13, "int": 13, "float": 13, "double": 13, "unsignedlong": 13} - {"index":{}} - {"id": 14, "halffloat": 14, "long": 14, "int": 14, "float": 14, "double": 14, "unsignedlong": 14} - {"index":{}} - {"id": 15, "halffloat": 15, "long": 15, "int": 15, "float": 15, "double": 15, "unsignedlong": 15} - {"index":{}} - {"id": 16, "halffloat": 16, "long": 16, "int": 16, "float": 16, "double": 16, "unsignedlong": 16} - {"index":{}} - {"id": 17, "halffloat": 17, "long": 17, "int": 17, "float": 17, "double": 17, "unsignedlong": 17} - {"index":{}} - {"id": 18, "halffloat": 18, "long": 18, "int": 18, "float": 18, "double": 18, "unsignedlong": 18} - {"index":{}} - {"id": 19, "halffloat": 19, "long": 19, "int": 19, "float": 19, "double": 19, "unsignedlong": 19} - {"index":{}} - {"id": 20, "halffloat": 20, "long": 20, "int": 20, "float": 20, "double": 20, "unsignedlong": 20} - {"index":{}} - {"id": 21, "halffloat": null, "long": null, "int": null, "float": null, "double": null, "unsignedlong": null} - {"index":{}} - {"id": 22, "halffloat": null, "long": null, "int": null, "float": null, "double": null, "unsignedlong": null} - {"index":{}} - {"id": 23, "halffloat": null, "long": null, "int": null, "float": null, "double": null, "unsignedlong": null} - {"index":{}} - {"id": 24, "halffloat": null, "long": null, "int": null, "float": null, "double": null, "unsignedlong": null} - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "halffloat": { "order": "asc" } }, { "id": { "order": "asc" } } ] - search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.halffloat: null } - - match: { hits.hits.0._source.id: 21 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.halffloat: null } - - match: { hits.hits.1._source.id: 22 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.halffloat: null } - - match: { hits.hits.2._source.id: 23 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "halffloat": { "order": "desc" } }, { "id": { "order": "desc" } } ] - search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.halffloat: null } - - match: { hits.hits.0._source.id: 24 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.halffloat: null } - - match: { hits.hits.1._source.id: 23 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.halffloat: null } - - match: { hits.hits.2._source.id: 22 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "long": { "order": "asc" } }, { "id": { "order": "asc" } } ] - search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.long: null } - - match: { hits.hits.0._source.id: 21 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.long: null } - - match: { hits.hits.1._source.id: 22 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.long: null } - - match: { hits.hits.2._source.id: 23 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "long": { "order": "desc" } }, { "id": { "order": "desc" } } ] - search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.long: null } - - match: { hits.hits.0._source.id: 24 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.long: null } - - match: { hits.hits.1._source.id: 23 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.long: null } - - match: { hits.hits.2._source.id: 22 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "int": { "order": "asc" } }, { "id": { "order": "asc" } } ] - search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.int: null } - - match: { hits.hits.0._source.id: 21 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.int: null } - - match: { hits.hits.1._source.id: 22 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.int: null } - - match: { hits.hits.2._source.id: 23 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "int": { "order": "desc" } }, { "id": { "order": "desc" } } ] - search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.int: null } - - match: { hits.hits.0._source.id: 24 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.int: null } - - match: { hits.hits.1._source.id: 23 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.int: null } - - match: { hits.hits.2._source.id: 22 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "float": { "order": "asc" } }, { "id": { "order": "asc" } } ] - search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.float: null } - - match: { hits.hits.0._source.id: 21 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.float: null } - - match: { hits.hits.1._source.id: 22 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.float: null } - - match: { hits.hits.2._source.id: 23 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "float": { "order": "desc" } }, { "id": { "order": "desc" } } ] - search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.float: null } - - match: { hits.hits.0._source.id: 24 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.float: null } - - match: { hits.hits.1._source.id: 23 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.float: null } - - match: { hits.hits.2._source.id: 22 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "double": { "order": "asc" } }, { "id": { "order": "asc" } } ] - search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.double: null } - - match: { hits.hits.0._source.id: 21 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.double: null } - - match: { hits.hits.1._source.id: 22 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.double: null } - - match: { hits.hits.2._source.id: 23 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "double": { "order": "desc" } }, { "id": { "order": "desc" } } ] - search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.double: null } - - match: { hits.hits.0._source.id: 24 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.double: null } - - match: { hits.hits.1._source.id: 23 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.double: null } - - match: { hits.hits.2._source.id: 22 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "unsignedlong": { "order": "asc" } }, { "id": { "order": "asc" } } ] - search_after: [ 200, 0 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.unsignedlong: null } - - match: { hits.hits.0._source.id: 21 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.unsignedlong: null } - - match: { hits.hits.1._source.id: 22 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.unsignedlong: null } - - match: { hits.hits.2._source.id: 23 } - - - do: - search: - index: test - rest_total_hits_as_int: true - body: - "size": 3 - "sort": [ { "unsignedlong": { "order": "desc" } }, { "id": { "order": "desc" } } ] - search_after: [ 0, 25 ] # making it out of min/max so only missing value hit is qualified - - - match: { hits.total: 24 } - - length: { hits.hits: 3 } - - match: { hits.hits.0._index: test } - - match: { hits.hits.0._source.unsignedlong: null } - - match: { hits.hits.0._source.id: 24 } - - match: { hits.hits.1._index: test } - - match: { hits.hits.1._source.unsignedlong: null } - - match: { hits.hits.1._source.id: 23 } - - match: { hits.hits.2._index: test } - - match: { hits.hits.2._source.unsignedlong: null } - - match: { hits.hits.2._source.id: 22 } 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 eb6fd2216d9b5..e70916c33882c 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, fieldname, dMissingValue, reversed, enableSkipping && this.enableSkipping) { + 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 e76e6ea1c6f08..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, fieldname, fMissingValue, reversed, enableSkipping && this.enableSkipping) { + 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/HalfFloatValuesComparatorSource.java b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java index 931d3423536b8..7e3936be1d8a5 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java +++ b/server/src/main/java/org/opensearch/index/fielddata/fieldcomparator/HalfFloatValuesComparatorSource.java @@ -48,7 +48,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 HalfFloatComparator(numHits, fieldname, fMissingValue, reversed, enableSkipping && this.enableSkipping) { + return new HalfFloatComparator(numHits, fieldname, null, reversed, enableSkipping && this.enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new HalfFloatLeafComparator(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 9dd95f79580cd..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, fieldname, iMissingValue, reversed, enableSkipping && this.enableSkipping) { + 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 60e978c4c1452..2c67d7ab7e86a 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, fieldname, lMissingValue, reversed, enableSkipping && this.enableSkipping) { + 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 9d1edde1fe08f..5416d4fe0fb91 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, fieldname, ulMissingValue, reversed, enableSkipping && this.enableSkipping) { + return new UnsignedLongComparator(numHits, fieldname, null, reversed, enableSkipping && this.enableSkipping) { @Override public LeafFieldComparator getLeafComparator(LeafReaderContext context) throws IOException { return new UnsignedLongLeafComparator(context) {