From 3bb0cc07f8e22b2b3f28360be1f67d2d255d5dcc Mon Sep 17 00:00:00 2001 From: gashutos Date: Thu, 14 Sep 2023 18:30:21 +0530 Subject: [PATCH 01/13] Fix broken backward comparibility from 2.7 for IndexSorted field indices Signed-off-by: gashutos --- .idea/runConfigurations/Debug_OpenSearch.xml | 11 ------- .../org/opensearch/index/IndexService.java | 1 + .../org/opensearch/index/IndexSettings.java | 16 ++++++++++ .../org/opensearch/index/IndexSortConfig.java | 7 ++++- .../index/fielddata/IndexFieldData.java | 7 +++++ .../fielddata/IndexNumericFieldData.java | 29 ++++++++++++++++++- .../search/sort/FieldSortBuilder.java | 1 + 7 files changed, 59 insertions(+), 13 deletions(-) delete mode 100644 .idea/runConfigurations/Debug_OpenSearch.xml diff --git a/.idea/runConfigurations/Debug_OpenSearch.xml b/.idea/runConfigurations/Debug_OpenSearch.xml deleted file mode 100644 index 0d8bf59823acf..0000000000000 --- a/.idea/runConfigurations/Debug_OpenSearch.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - diff --git a/server/src/main/java/org/opensearch/index/IndexService.java b/server/src/main/java/org/opensearch/index/IndexService.java index 80ead0a333ba3..e376fcba4f5c4 100644 --- a/server/src/main/java/org/opensearch/index/IndexService.java +++ b/server/src/main/java/org/opensearch/index/IndexService.java @@ -242,6 +242,7 @@ public IndexService( // The sort order is validated right after the merge of the mapping later in the process. this.indexSortSupplier = () -> indexSettings.getIndexSortConfig() .buildIndexSort( + this.indexSettings.shouldWidenIndexSortType(), mapperService::fieldType, (fieldType, searchLookup) -> indexFieldData.getForField(fieldType, indexFieldData.index().getName(), searchLookup) ); diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index 03c71351294d5..e794732cf463f 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -660,6 +660,7 @@ public final class IndexSettings { private volatile long retentionLeaseMillis; private volatile String defaultSearchPipeline; + private final boolean widenIndexSortType; /** * The maximum age of a retention lease before it is considered expired. @@ -857,6 +858,13 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti mergeOnFlushEnabled = scopedSettings.get(INDEX_MERGE_ON_FLUSH_ENABLED); setMergeOnFlushPolicy(scopedSettings.get(INDEX_MERGE_ON_FLUSH_POLICY)); defaultSearchPipeline = scopedSettings.get(DEFAULT_SEARCH_PIPELINE); + /* There was unintentional breaking change got introduced with OpenSearch-6424 (version 2.7). + * For indices created prior version (prior to 2.7) which has IndexSort type, they used to type caste the SortField.Type + * to higher bytes size like integer to long. This behavior was changed from OpenSearch 2.7 version not to + * up caste the SortField to gain some sort query optimizations. + * Now this sortField (IndexSort) is stored in SegmentInfo and we need to maintain backward compatibility for them. + */ + widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).onOrBefore(LegacyESVersion.V_2_6_1); scopedSettings.addSettingsUpdateConsumer(MergePolicyConfig.INDEX_COMPOUND_FORMAT_SETTING, mergePolicyConfig::setNoCFSRatio); scopedSettings.addSettingsUpdateConsumer( @@ -1652,4 +1660,12 @@ public String getDefaultSearchPipeline() { public void setDefaultSearchPipeline(String defaultSearchPipeline) { this.defaultSearchPipeline = defaultSearchPipeline; } + + /** + * Returns true if we need to maintain backward compatibility for index sorted indices created prior to version 2.7 + * @return boolean + */ + public boolean shouldWidenIndexSortType() { + return this.widenIndexSortType; + } } diff --git a/server/src/main/java/org/opensearch/index/IndexSortConfig.java b/server/src/main/java/org/opensearch/index/IndexSortConfig.java index f73f96df4f9ad..c4b5d2f3bf261 100644 --- a/server/src/main/java/org/opensearch/index/IndexSortConfig.java +++ b/server/src/main/java/org/opensearch/index/IndexSortConfig.java @@ -200,6 +200,7 @@ public boolean hasPrimarySortOnField(String field) { * or returns null if this index has no sort. */ public Sort buildIndexSort( + boolean shouldWidenIndexSortTpe, Function fieldTypeLookup, BiFunction, IndexFieldData> fieldDataLookup ) { @@ -230,7 +231,11 @@ public Sort buildIndexSort( if (fieldData == null) { throw new IllegalArgumentException("docvalues not found for index sort field:[" + sortSpec.field + "]"); } - sortFields[i] = fieldData.sortField(sortSpec.missingValue, mode, null, reverse); + if (shouldWidenIndexSortTpe == true) { + sortFields[i] = fieldData.wideSortField(sortSpec.missingValue, mode, null, reverse); + } else { + sortFields[i] = fieldData.sortField(sortSpec.missingValue, mode, null, reverse); + } validateIndexSortField(sortFields[i]); } return new Sort(sortFields); 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 f9db28a2c56fe..81d4ce2dd8772 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/IndexFieldData.java +++ b/server/src/main/java/org/opensearch/index/fielddata/IndexFieldData.java @@ -94,6 +94,13 @@ public interface IndexFieldData { */ SortField sortField(@Nullable Object missingValue, MultiValueMode sortMode, Nested nested, boolean reverse); + /** + * Returns the {@link SortField} to use for index sorting where we widen the sort field type to higher or equal bytes. + */ + default SortField wideSortField(@Nullable Object missingValue, MultiValueMode sortMode, Nested nested, boolean reverse) { + return sortField(missingValue, sortMode, nested, reverse); + } + /** * Build a sort implementation specialized for aggregations. */ 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 ae8ffd8fe6b97..480128bfdfef3 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java @@ -81,12 +81,15 @@ public enum NumericType { private final boolean floatingPoint; private final ValuesSourceType valuesSourceType; - private final SortField.Type sortFieldType; + private SortField.Type sortFieldType; + + private boolean usePointBasedOptimization; NumericType(boolean floatingPoint, SortField.Type sortFieldType, ValuesSourceType valuesSourceType) { this.floatingPoint = floatingPoint; this.sortFieldType = sortFieldType; this.valuesSourceType = valuesSourceType; + this.usePointBasedOptimization = true; } public final boolean isFloatingPoint() { @@ -96,6 +99,12 @@ public final boolean isFloatingPoint() { public final ValuesSourceType getValuesSourceType() { return valuesSourceType; } + + public void setSortFieldType(SortField.Type type) { + this.sortFieldType = type; + this.usePointBasedOptimization = false; // Disable optimization if we set this + // This means indexed point type might be different compare to mapping type + } } /** @@ -128,6 +137,7 @@ public final SortField sortField( || nested != null || (sortMode != MultiValueMode.MAX && sortMode != MultiValueMode.MIN) || targetNumericType != getNumericType()) { + System.out.println("Custom comparator logic....."); return new SortField(getFieldName(), source, reverse); } @@ -136,6 +146,9 @@ public final SortField sortField( : SortedNumericSelector.Type.MIN; SortField sortField = new SortedNumericSortField(getFieldName(), getNumericType().sortFieldType, reverse, selectorType); sortField.setMissingValue(source.missingObject(missingValue, reverse)); + if (getNumericType().usePointBasedOptimization == false) { + sortField.setOptimizeSortWithPoints(false); + } return sortField; } @@ -151,6 +164,16 @@ public final SortField sortField(Object missingValue, MultiValueMode sortMode, N return sortField(getNumericType(), missingValue, sortMode, nested, reverse); } + @Override + public final SortField wideSortField(Object missingValue, MultiValueMode sortMode, Nested nested, boolean reverse) { + switch (getNumericType().sortFieldType) { + case INT: + getNumericType().setSortFieldType(NumericType.LONG.sortFieldType); + break; + } + return sortField(getNumericType(), missingValue, sortMode, nested, reverse); + } + /** * Builds a {@linkplain BucketedSort} for the {@code targetNumericType}, * casting the values if their native type doesn't match. @@ -220,6 +243,10 @@ private XFieldComparatorSource comparatorSource( source = new LongValuesComparatorSource(this, missingValue, sortMode, nested); break; default: + if (getNumericType().sortFieldType == SortField.Type.LONG) { + // Always consider Long source for sortField.Type Long in default + return new LongValuesComparatorSource(this, missingValue, sortMode, nested); + } assert !targetNumericType.isFloatingPoint(); source = new IntValuesComparatorSource(this, missingValue, sortMode, nested); } diff --git a/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java index 97e1d444d4a0a..6f97fa0e68526 100644 --- a/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java @@ -428,6 +428,7 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException { } IndexNumericFieldData numericFieldData = (IndexNumericFieldData) fieldData; NumericType resolvedType = resolveNumericType(numericType); + System.out.println("CHETAN LOGS : " + resolvedType); field = numericFieldData.sortField(resolvedType, missing, localSortMode(), nested, reverse); isNanosecond = resolvedType == NumericType.DATE_NANOSECONDS; } else { From f3ff4c1dbc68a30ed11fa3d7610e474961267992 Mon Sep 17 00:00:00 2001 From: gashutos Date: Thu, 14 Sep 2023 18:47:30 +0530 Subject: [PATCH 02/13] Adding CHANGELOG Signed-off-by: gashutos --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a52c3d7f2a3f..e734834c70825 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Fixed - Fix ignore_missing parameter has no effect when using template snippet in rename ingest processor ([#9725](https://github.com/opensearch-project/OpenSearch/pull/9725)) +- Fix broken backward compatibility from 2.7 for IndexSorted field indices ([#10045](https://github.com/opensearch-project/OpenSearch/pull/9725)) ### Security From 7d4359ac9025eaa7cf2a1f3ac8b8126f89c65dda Mon Sep 17 00:00:00 2001 From: Chaitanya Gohel <104654647+gashutos@users.noreply.github.com> Date: Thu, 14 Sep 2023 18:49:01 +0530 Subject: [PATCH 03/13] Update server/src/main/java/org/opensearch/index/IndexSettings.java Co-authored-by: Andriy Redko Signed-off-by: Chaitanya Gohel <104654647+gashutos@users.noreply.github.com> --- server/src/main/java/org/opensearch/index/IndexSettings.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index e794732cf463f..d4183639a5bfc 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -858,7 +858,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti mergeOnFlushEnabled = scopedSettings.get(INDEX_MERGE_ON_FLUSH_ENABLED); setMergeOnFlushPolicy(scopedSettings.get(INDEX_MERGE_ON_FLUSH_POLICY)); defaultSearchPipeline = scopedSettings.get(DEFAULT_SEARCH_PIPELINE); - /* There was unintentional breaking change got introduced with OpenSearch-6424 (version 2.7). + /* There was unintentional breaking change got introduced with [OpenSearch-6424](https://github.com/opensearch-project/OpenSearch/pull/6424) (version 2.7). * For indices created prior version (prior to 2.7) which has IndexSort type, they used to type caste the SortField.Type * to higher bytes size like integer to long. This behavior was changed from OpenSearch 2.7 version not to * up caste the SortField to gain some sort query optimizations. From 97d650fa67541122620534eb8488c7baec1a374c Mon Sep 17 00:00:00 2001 From: gashutos Date: Thu, 14 Sep 2023 18:59:13 +0530 Subject: [PATCH 04/13] Removing unwanted logs Signed-off-by: gashutos --- server/src/main/java/org/opensearch/index/IndexSettings.java | 1 + .../main/java/org/opensearch/search/sort/FieldSortBuilder.java | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index d4183639a5bfc..bc85c0ab364a7 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -34,6 +34,7 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.index.MergePolicy; import org.apache.lucene.sandbox.index.MergeOnFlushMergePolicy; +import org.opensearch.LegacyESVersion; import org.opensearch.Version; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.annotation.PublicApi; diff --git a/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java index 6f97fa0e68526..97e1d444d4a0a 100644 --- a/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java @@ -428,7 +428,6 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException { } IndexNumericFieldData numericFieldData = (IndexNumericFieldData) fieldData; NumericType resolvedType = resolveNumericType(numericType); - System.out.println("CHETAN LOGS : " + resolvedType); field = numericFieldData.sortField(resolvedType, missing, localSortMode(), nested, reverse); isNanosecond = resolvedType == NumericType.DATE_NANOSECONDS; } else { From 9e9ae13b6b675e2e7cfbdc57698bbdf399014bea Mon Sep 17 00:00:00 2001 From: gashutos Date: Thu, 14 Sep 2023 19:30:29 +0530 Subject: [PATCH 05/13] Removing unwanted logs Signed-off-by: gashutos --- .idea/runConfigurations/Debug_OpenSearch.xml | 11 +++++++++++ .../main/java/org/opensearch/index/IndexSettings.java | 4 ++-- .../index/fielddata/IndexNumericFieldData.java | 1 - 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 .idea/runConfigurations/Debug_OpenSearch.xml diff --git a/.idea/runConfigurations/Debug_OpenSearch.xml b/.idea/runConfigurations/Debug_OpenSearch.xml new file mode 100644 index 0000000000000..0d8bf59823acf --- /dev/null +++ b/.idea/runConfigurations/Debug_OpenSearch.xml @@ -0,0 +1,11 @@ + + + + diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index bc85c0ab364a7..1ed8995805a0c 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -34,7 +34,6 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.index.MergePolicy; import org.apache.lucene.sandbox.index.MergeOnFlushMergePolicy; -import org.opensearch.LegacyESVersion; import org.opensearch.Version; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.annotation.PublicApi; @@ -64,6 +63,7 @@ import java.util.function.Function; import java.util.function.UnaryOperator; +import static org.opensearch.Version.V_2_6_1; import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING; @@ -865,7 +865,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti * up caste the SortField to gain some sort query optimizations. * Now this sortField (IndexSort) is stored in SegmentInfo and we need to maintain backward compatibility for them. */ - widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).onOrBefore(LegacyESVersion.V_2_6_1); + widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).onOrBefore(V_2_6_1); scopedSettings.addSettingsUpdateConsumer(MergePolicyConfig.INDEX_COMPOUND_FORMAT_SETTING, mergePolicyConfig::setNoCFSRatio); scopedSettings.addSettingsUpdateConsumer( 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 480128bfdfef3..50251284d8736 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java @@ -137,7 +137,6 @@ public final SortField sortField( || nested != null || (sortMode != MultiValueMode.MAX && sortMode != MultiValueMode.MIN) || targetNumericType != getNumericType()) { - System.out.println("Custom comparator logic....."); return new SortField(getFieldName(), source, reverse); } From 51ed42215476ec51997b1ae1626f6aa691b08589 Mon Sep 17 00:00:00 2001 From: gashutos Date: Thu, 14 Sep 2023 22:21:12 +0530 Subject: [PATCH 06/13] Adding index sort as part of mixed cluster to test this scenario Signed-off-by: gashutos --- .../org/opensearch/backwards/IndexingIT.java | 34 +++++++++++++++---- .../org/opensearch/index/IndexService.java | 1 - .../org/opensearch/index/IndexSortConfig.java | 5 +-- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java b/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java index 686fc78dcec8a..77f4102a95cb9 100644 --- a/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java +++ b/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java @@ -46,6 +46,7 @@ import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.common.xcontent.support.XContentMapValues; import org.opensearch.core.common.Strings; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.index.seqno.SeqNoStats; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.core.rest.RestStatus; @@ -63,17 +64,19 @@ import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; +import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; public class IndexingIT extends OpenSearchRestTestCase { protected static final Version UPGRADE_FROM_VERSION = Version.fromString(System.getProperty("tests.upgrade_from_version")); + private static final String TEST_MAPPING = createTestMapping(); private int indexDocs(String index, final int idStart, final int numDocs) throws IOException { for (int i = 0; i < numDocs; i++) { final int id = idStart + i; Request request = new Request("PUT", index + "/_doc/" + id); - request.setJsonEntity("{\"test\": \"test_" + randomAlphaOfLength(2) + "\"}"); + request.setJsonEntity("{\"test\": \"test_" + randomAlphaOfLength(2) + "\", \"sortfield\": \""+ randomIntBetween(0, numDocs) + "\"}"); assertOK(client().performRequest(request)); } return numDocs; @@ -129,9 +132,10 @@ public void testIndexingWithPrimaryOnBwcNodes() throws Exception { .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1) .put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 0) .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT) + .putList("index.sort.field", "sortfield") .put("index.routing.allocation.include._name", bwcNames); final String index = "test-index"; - createIndex(index, settings.build()); + createIndex(index, settings.build(), TEST_MAPPING); ensureNoInitializingShards(); // wait for all other shard activity to finish int docCount = 200; @@ -178,9 +182,10 @@ public void testIndexingWithReplicaOnBwcNodes() throws Exception { .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1) .put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 0) .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT) + .putList("index.sort.field", "sortfield") .put("index.routing.allocation.exclude._name", bwcNames); final String index = "test-index"; - createIndex(index, settings.build()); + createIndex(index, settings.build(), TEST_MAPPING); ensureNoInitializingShards(); // wait for all other shard activity to finish printClusterRouting(); @@ -214,11 +219,12 @@ public void testIndexVersionPropagation() throws Exception { Settings.Builder settings = Settings.builder() .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1) .put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 2) + .putList("index.sort.field", "sortfield") .put("index.routing.allocation.include._name", bwcNames); final String index = "indexversionprop"; final int minUpdates = 5; final int maxUpdates = 10; - createIndex(index, settings.build()); + createIndex(index, settings.build(), TEST_MAPPING); try (RestClient newNodeClient = buildClient(restClientSettings(), nodes.getNewNodes().stream().map(Node::getPublishAddress).toArray(HttpHost[]::new))) { @@ -300,10 +306,11 @@ public void testSeqNoCheckpoints() throws Exception { Settings.Builder settings = Settings.builder() .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1) .put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 2) + .putList("index.sort.field", "sortfield") .put("index.routing.allocation.include._name", bwcNames); final String index = "test"; - createIndex(index, settings.build()); + createIndex(index, settings.build(), TEST_MAPPING); try (RestClient newNodeClient = buildClient(restClientSettings(), nodes.getNewNodes().stream().map(Node::getPublishAddress).toArray(HttpHost[]::new))) { int numDocs = 0; @@ -382,10 +389,11 @@ public void testUpdateSnapshotStatus() throws Exception { Settings.Builder settings = Settings.builder() .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), between(5, 10)) .put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 1) + .putList("index.sort.field", "sortfield") .put("index.routing.allocation.include._name", bwcNames); final String index = "test-snapshot-index"; - createIndex(index, settings.build()); + createIndex(index, settings.build(), TEST_MAPPING); indexDocs(index, 0, between(50, 100)); ensureGreen(index); assertOK(client().performRequest(new Request("POST", index + "/_refresh"))); @@ -419,7 +427,8 @@ public void testSyncedFlushTransition() throws Exception { createIndex(index, Settings.builder() .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), numShards) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numOfReplicas) - .put("index.routing.allocation.include._name", newNodes).build()); + .putList("index.sort.field", "sortfield") + .put("index.routing.allocation.include._name", newNodes).build(), TEST_MAPPING); ensureGreen(index); indexDocs(index, randomIntBetween(0, 100), between(1, 100)); try (RestClient oldNodeClient = buildClient(restClientSettings(), @@ -664,4 +673,15 @@ public String toString() { '}'; } } + + private static String createTestMapping() { + return " \"properties\": {\n" + + " \"test\": {\n" + + " \"type\": \"text\"\n" + + " },\n" + + " \"sortfield\": {\n" + + " \"type\": \"integer\"\n" + + " }\n" + + " }"; + } } diff --git a/server/src/main/java/org/opensearch/index/IndexService.java b/server/src/main/java/org/opensearch/index/IndexService.java index e376fcba4f5c4..80ead0a333ba3 100644 --- a/server/src/main/java/org/opensearch/index/IndexService.java +++ b/server/src/main/java/org/opensearch/index/IndexService.java @@ -242,7 +242,6 @@ public IndexService( // The sort order is validated right after the merge of the mapping later in the process. this.indexSortSupplier = () -> indexSettings.getIndexSortConfig() .buildIndexSort( - this.indexSettings.shouldWidenIndexSortType(), mapperService::fieldType, (fieldType, searchLookup) -> indexFieldData.getForField(fieldType, indexFieldData.index().getName(), searchLookup) ); diff --git a/server/src/main/java/org/opensearch/index/IndexSortConfig.java b/server/src/main/java/org/opensearch/index/IndexSortConfig.java index c4b5d2f3bf261..bbed50badad83 100644 --- a/server/src/main/java/org/opensearch/index/IndexSortConfig.java +++ b/server/src/main/java/org/opensearch/index/IndexSortConfig.java @@ -143,6 +143,7 @@ private static MultiValueMode parseMultiValueMode(String value) { // visible for tests final FieldSortSpec[] sortSpecs; + final boolean shouldWidenIndexSortTpe; public IndexSortConfig(IndexSettings indexSettings) { final Settings settings = indexSettings.getSettings(); @@ -182,6 +183,7 @@ public IndexSortConfig(IndexSettings indexSettings) { sortSpecs[i].missingValue = missingValues.get(i); } } + this.shouldWidenIndexSortTpe = indexSettings.shouldWidenIndexSortType(); } /** @@ -200,7 +202,6 @@ public boolean hasPrimarySortOnField(String field) { * or returns null if this index has no sort. */ public Sort buildIndexSort( - boolean shouldWidenIndexSortTpe, Function fieldTypeLookup, BiFunction, IndexFieldData> fieldDataLookup ) { @@ -231,7 +232,7 @@ public Sort buildIndexSort( if (fieldData == null) { throw new IllegalArgumentException("docvalues not found for index sort field:[" + sortSpec.field + "]"); } - if (shouldWidenIndexSortTpe == true) { + if (this.shouldWidenIndexSortTpe == true) { sortFields[i] = fieldData.wideSortField(sortSpec.missingValue, mode, null, reverse); } else { sortFields[i] = fieldData.sortField(sortSpec.missingValue, mode, null, reverse); From b037213d224518d450425310d041f0e42938c827 Mon Sep 17 00:00:00 2001 From: gashutos Date: Fri, 15 Sep 2023 02:16:21 +0530 Subject: [PATCH 07/13] Removing optimization disable logic Signed-off-by: gashutos --- .../fielddata/IndexNumericFieldData.java | 35 ++++++++----------- .../AbstractFieldDataImplTestCase.java | 19 ++++++++++ 2 files changed, 33 insertions(+), 21 deletions(-) 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 50251284d8736..9ed562eba4886 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java @@ -81,15 +81,12 @@ public enum NumericType { private final boolean floatingPoint; private final ValuesSourceType valuesSourceType; - private SortField.Type sortFieldType; - - private boolean usePointBasedOptimization; + private final SortField.Type sortFieldType; NumericType(boolean floatingPoint, SortField.Type sortFieldType, ValuesSourceType valuesSourceType) { this.floatingPoint = floatingPoint; this.sortFieldType = sortFieldType; this.valuesSourceType = valuesSourceType; - this.usePointBasedOptimization = true; } public final boolean isFloatingPoint() { @@ -99,12 +96,6 @@ public final boolean isFloatingPoint() { public final ValuesSourceType getValuesSourceType() { return valuesSourceType; } - - public void setSortFieldType(SortField.Type type) { - this.sortFieldType = type; - this.usePointBasedOptimization = false; // Disable optimization if we set this - // This means indexed point type might be different compare to mapping type - } } /** @@ -145,9 +136,6 @@ public final SortField sortField( : SortedNumericSelector.Type.MIN; SortField sortField = new SortedNumericSortField(getFieldName(), getNumericType().sortFieldType, reverse, selectorType); sortField.setMissingValue(source.missingObject(missingValue, reverse)); - if (getNumericType().usePointBasedOptimization == false) { - sortField.setOptimizeSortWithPoints(false); - } return sortField; } @@ -165,11 +153,20 @@ public final SortField sortField(Object missingValue, MultiValueMode sortMode, N @Override public final SortField wideSortField(Object missingValue, MultiValueMode sortMode, Nested nested, boolean reverse) { - switch (getNumericType().sortFieldType) { - case INT: - getNumericType().setSortFieldType(NumericType.LONG.sortFieldType); - break; + // This is to support backward compatibility, the minimum number of bytes prior to OpenSearch 2.7 were 8 bytes, + // i.e all sort fields were upcasted to Long/Double with 16 bytes. + // Now from OpenSearch 2.7, the minimum number of bytes for sort field is 8 bytes, so if it comes as SortField INT, + // we need to up caste it to LONG to support backward compatibility info stored in segment info + if (getNumericType().sortFieldType == SortField.Type.INT) { + XFieldComparatorSource source = comparatorSource(NumericType.LONG, missingValue, sortMode, nested); + SortedNumericSelector.Type selectorType = sortMode == MultiValueMode.MAX + ? SortedNumericSelector.Type.MAX + : SortedNumericSelector.Type.MIN; + SortField sortField = new SortedNumericSortField(getFieldName(), SortField.Type.LONG, reverse, selectorType); + sortField.setMissingValue(source.missingObject(missingValue, reverse)); + return sortField; } + // If already more than INT, up caste not needed. return sortField(getNumericType(), missingValue, sortMode, nested, reverse); } @@ -242,10 +239,6 @@ private XFieldComparatorSource comparatorSource( source = new LongValuesComparatorSource(this, missingValue, sortMode, nested); break; default: - if (getNumericType().sortFieldType == SortField.Type.LONG) { - // Always consider Long source for sortField.Type Long in default - return new LongValuesComparatorSource(this, missingValue, sortMode, nested); - } assert !targetNumericType.isFloatingPoint(); source = new IntValuesComparatorSource(this, missingValue, sortMode, nested); } diff --git a/server/src/test/java/org/opensearch/index/fielddata/AbstractFieldDataImplTestCase.java b/server/src/test/java/org/opensearch/index/fielddata/AbstractFieldDataImplTestCase.java index 1ffacf98a6836..8bcd6ba804237 100644 --- a/server/src/test/java/org/opensearch/index/fielddata/AbstractFieldDataImplTestCase.java +++ b/server/src/test/java/org/opensearch/index/fielddata/AbstractFieldDataImplTestCase.java @@ -39,6 +39,7 @@ import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Sort; import org.apache.lucene.search.SortField; +import org.apache.lucene.search.SortedNumericSortField; import org.apache.lucene.search.TopFieldDocs; import org.apache.lucene.util.BytesRef; import org.opensearch.core.common.Strings; @@ -144,6 +145,24 @@ public void testSingleValueAllSet() throws Exception { } } + public void testWideSortField() throws Exception { + // integer to long widening should happen + IndexFieldData indexFieldData = getForField("integer", "value"); + SortField sortField = indexFieldData.wideSortField(null, MultiValueMode.MIN, null, false); + assertTrue(((SortedNumericSortField) sortField).getNumericType() == SortField.Type.LONG); + + // long to long no widening should happen + indexFieldData = getForField("long", "value"); + sortField = indexFieldData.wideSortField(null, MultiValueMode.MIN, null, false); + assertTrue(((SortedNumericSortField) sortField).getNumericType() == SortField.Type.LONG); + + // float to float no widening should happen + indexFieldData = getForField("float", "value"); + sortField = indexFieldData.wideSortField(null, MultiValueMode.MIN, null, false); + assertTrue(((SortedNumericSortField) sortField).getNumericType() == SortField.Type.FLOAT); + + } + protected abstract void fillSingleValueWithMissing() throws Exception; public void assertValues(SortedBinaryDocValues values, int docId, BytesRef... actualValues) throws IOException { From a7e09c63c17e84c29acb0ddc76ac4f5da247795f Mon Sep 17 00:00:00 2001 From: gashutos Date: Fri, 15 Sep 2023 09:53:55 +0530 Subject: [PATCH 08/13] Correcting some comments & version check to before( V_2_7_0) instead onOrBefire(V_2_6_1) since Signed-off-by: gashutos --- server/src/main/java/org/opensearch/index/IndexSettings.java | 3 ++- .../org/opensearch/index/fielddata/IndexNumericFieldData.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index 1ed8995805a0c..03b6b80c2ee60 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -64,6 +64,7 @@ import java.util.function.UnaryOperator; import static org.opensearch.Version.V_2_6_1; +import static org.opensearch.Version.V_2_7_0; import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING; @@ -865,7 +866,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti * up caste the SortField to gain some sort query optimizations. * Now this sortField (IndexSort) is stored in SegmentInfo and we need to maintain backward compatibility for them. */ - widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).onOrBefore(V_2_6_1); + widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).before(V_2_7_0); scopedSettings.addSettingsUpdateConsumer(MergePolicyConfig.INDEX_COMPOUND_FORMAT_SETTING, mergePolicyConfig::setNoCFSRatio); scopedSettings.addSettingsUpdateConsumer( 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 9ed562eba4886..dda0b4ff72b67 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java @@ -153,7 +153,7 @@ public final SortField sortField(Object missingValue, MultiValueMode sortMode, N @Override public final SortField wideSortField(Object missingValue, MultiValueMode sortMode, Nested nested, boolean reverse) { - // This is to support backward compatibility, the minimum number of bytes prior to OpenSearch 2.7 were 8 bytes, + // This is to support backward compatibility, the minimum number of bytes prior to OpenSearch 2.7 were 16 bytes, // i.e all sort fields were upcasted to Long/Double with 16 bytes. // Now from OpenSearch 2.7, the minimum number of bytes for sort field is 8 bytes, so if it comes as SortField INT, // we need to up caste it to LONG to support backward compatibility info stored in segment info From 7f81045ab3dccc0db797dd6cbf838480c63c24da Mon Sep 17 00:00:00 2001 From: gashutos Date: Fri, 15 Sep 2023 10:11:15 +0530 Subject: [PATCH 09/13] Resolving spotless check error Signed-off-by: gashutos --- server/src/main/java/org/opensearch/index/IndexSettings.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index 03b6b80c2ee60..9b5337948e278 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -63,7 +63,6 @@ import java.util.function.Function; import java.util.function.UnaryOperator; -import static org.opensearch.Version.V_2_6_1; import static org.opensearch.Version.V_2_7_0; import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING; From 2575a45391ba5878370a756d5a925f500ce0ff81 Mon Sep 17 00:00:00 2001 From: gashutos Date: Fri, 15 Sep 2023 12:29:52 +0530 Subject: [PATCH 10/13] Fixing broken UT - last minute checkin without copile Signed-off-by: gashutos --- .../index/fielddata/AbstractFieldDataImplTestCase.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/index/fielddata/AbstractFieldDataImplTestCase.java b/server/src/test/java/org/opensearch/index/fielddata/AbstractFieldDataImplTestCase.java index 8bcd6ba804237..2b44e759f4ff9 100644 --- a/server/src/test/java/org/opensearch/index/fielddata/AbstractFieldDataImplTestCase.java +++ b/server/src/test/java/org/opensearch/index/fielddata/AbstractFieldDataImplTestCase.java @@ -146,8 +146,11 @@ public void testSingleValueAllSet() throws Exception { } public void testWideSortField() throws Exception { + if (this instanceof NoOrdinalsStringFieldDataTests || this instanceof PagedBytesStringFieldDataTests) { + return; // Numeric types are not supported there. + } // integer to long widening should happen - IndexFieldData indexFieldData = getForField("integer", "value"); + IndexFieldData indexFieldData = getForField("int", "value"); SortField sortField = indexFieldData.wideSortField(null, MultiValueMode.MIN, null, false); assertTrue(((SortedNumericSortField) sortField).getNumericType() == SortField.Type.LONG); From f61aba62f0eb6de3092d66827fd881caf75cec6f Mon Sep 17 00:00:00 2001 From: gashutos Date: Fri, 15 Sep 2023 14:01:32 +0530 Subject: [PATCH 11/13] Improving code coverage to make codcov happy Signed-off-by: gashutos --- .../opensearch/index/IndexServiceTests.java | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/server/src/test/java/org/opensearch/index/IndexServiceTests.java b/server/src/test/java/org/opensearch/index/IndexServiceTests.java index 1b8e1abb1bf1b..a9fc108f6addb 100644 --- a/server/src/test/java/org/opensearch/index/IndexServiceTests.java +++ b/server/src/test/java/org/opensearch/index/IndexServiceTests.java @@ -33,7 +33,9 @@ package org.opensearch.index; import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.SortField; import org.apache.lucene.search.TopDocs; +import org.opensearch.Version; import org.opensearch.action.support.ActiveShardCount; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.compress.CompressedXContent; @@ -526,4 +528,60 @@ public void testUpdateRemoteTranslogBufferIntervalDynamically() { indexMetadata = client().admin().cluster().prepareState().execute().actionGet().getState().metadata().index("test"); assertEquals("20s", indexMetadata.getSettings().get(IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.getKey())); } + + public void testIndexSort() { + Settings settings = Settings.builder() + .put(IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING.getKey(), "0ms") // disable + .putList("index.sort.field", "sortfield") + .build(); + try { + // Integer index sort should be remained to int sort type + IndexService index = createIndex("test", settings, createTestMapping("integer")); + assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.INT); + + // Long index sort should be remained to long sort type + index = createIndex("test", settings, createTestMapping("long")); + assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.LONG); + + // String index sort should be remained to string sort type + index = createIndex("test", settings, createTestMapping("string")); + assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.STRING); + } catch (IllegalArgumentException ex) { + assertEquals("failed to parse value [0ms] for setting [index.translog.sync_interval], must be >= [100ms]", ex.getMessage()); + } + } + + public void testIndexSortBackwardCompatible() { + Settings settings = Settings.builder() + .put(IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING.getKey(), "0ms") // disable + .put(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(), Version.V_2_6_1) + .putList("index.sort.field", "sortfield") + .build(); + try { + // Integer index sort should be converted to long sort type + IndexService index = createIndex("test", settings, createTestMapping("integer")); + assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.LONG); + + // Long index sort should be remained to long sort type + index = createIndex("test", settings, createTestMapping("long")); + assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.LONG); + + // String index sort should be remained to string sort type + index = createIndex("test", settings, createTestMapping("string")); + assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.STRING); + } catch (IllegalArgumentException ex) { + assertEquals("failed to parse value [0ms] for setting [index.translog.sync_interval], must be >= [100ms]", ex.getMessage()); + } + } + + private static String createTestMapping(String type) { + return " \"properties\": {\n" + + " \"test\": {\n" + + " \"type\": \"text\"\n" + + " },\n" + + " \"sortfield\": {\n" + + " \"type\": \" + type + \"\n" + + " }\n" + + " }"; + } } From 61d355ed9949bd430cf14b1582d0343762c4ee4b Mon Sep 17 00:00:00 2001 From: gashutos Date: Fri, 15 Sep 2023 18:56:14 +0530 Subject: [PATCH 12/13] Correcting typos and adding more tests Signed-off-by: gashutos --- .../java/org/opensearch/index/IndexSettings.java | 4 ++-- .../org/opensearch/index/IndexSortConfig.java | 6 +++--- .../index/fielddata/IndexNumericFieldData.java | 6 +++--- .../org/opensearch/index/IndexServiceTests.java | 16 ++++++++++++++++ 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index 9b5337948e278..1e4224c314f05 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -860,9 +860,9 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti setMergeOnFlushPolicy(scopedSettings.get(INDEX_MERGE_ON_FLUSH_POLICY)); defaultSearchPipeline = scopedSettings.get(DEFAULT_SEARCH_PIPELINE); /* There was unintentional breaking change got introduced with [OpenSearch-6424](https://github.com/opensearch-project/OpenSearch/pull/6424) (version 2.7). - * For indices created prior version (prior to 2.7) which has IndexSort type, they used to type caste the SortField.Type + * For indices created prior version (prior to 2.7) which has IndexSort type, they used to type cast the SortField.Type * to higher bytes size like integer to long. This behavior was changed from OpenSearch 2.7 version not to - * up caste the SortField to gain some sort query optimizations. + * up cast the SortField to gain some sort query optimizations. * Now this sortField (IndexSort) is stored in SegmentInfo and we need to maintain backward compatibility for them. */ widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).before(V_2_7_0); diff --git a/server/src/main/java/org/opensearch/index/IndexSortConfig.java b/server/src/main/java/org/opensearch/index/IndexSortConfig.java index bbed50badad83..83192052564f3 100644 --- a/server/src/main/java/org/opensearch/index/IndexSortConfig.java +++ b/server/src/main/java/org/opensearch/index/IndexSortConfig.java @@ -143,7 +143,7 @@ private static MultiValueMode parseMultiValueMode(String value) { // visible for tests final FieldSortSpec[] sortSpecs; - final boolean shouldWidenIndexSortTpe; + final boolean shouldWidenIndexSortType; public IndexSortConfig(IndexSettings indexSettings) { final Settings settings = indexSettings.getSettings(); @@ -183,7 +183,7 @@ public IndexSortConfig(IndexSettings indexSettings) { sortSpecs[i].missingValue = missingValues.get(i); } } - this.shouldWidenIndexSortTpe = indexSettings.shouldWidenIndexSortType(); + this.shouldWidenIndexSortType = indexSettings.shouldWidenIndexSortType(); } /** @@ -232,7 +232,7 @@ public Sort buildIndexSort( if (fieldData == null) { throw new IllegalArgumentException("docvalues not found for index sort field:[" + sortSpec.field + "]"); } - if (this.shouldWidenIndexSortTpe == true) { + if (this.shouldWidenIndexSortType == true) { sortFields[i] = fieldData.wideSortField(sortSpec.missingValue, mode, null, reverse); } else { sortFields[i] = fieldData.sortField(sortSpec.missingValue, mode, null, reverse); 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 dda0b4ff72b67..b4e90b8ab570a 100644 --- a/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/opensearch/index/fielddata/IndexNumericFieldData.java @@ -156,7 +156,7 @@ public final SortField wideSortField(Object missingValue, MultiValueMode sortMod // This is to support backward compatibility, the minimum number of bytes prior to OpenSearch 2.7 were 16 bytes, // i.e all sort fields were upcasted to Long/Double with 16 bytes. // Now from OpenSearch 2.7, the minimum number of bytes for sort field is 8 bytes, so if it comes as SortField INT, - // we need to up caste it to LONG to support backward compatibility info stored in segment info + // we need to up cast it to LONG to support backward compatibility info stored in segment info if (getNumericType().sortFieldType == SortField.Type.INT) { XFieldComparatorSource source = comparatorSource(NumericType.LONG, missingValue, sortMode, nested); SortedNumericSelector.Type selectorType = sortMode == MultiValueMode.MAX @@ -166,7 +166,7 @@ public final SortField wideSortField(Object missingValue, MultiValueMode sortMod sortField.setMissingValue(source.missingObject(missingValue, reverse)); return sortField; } - // If already more than INT, up caste not needed. + // If already more than INT, up cast not needed. return sortField(getNumericType(), missingValue, sortMode, nested, reverse); } @@ -243,7 +243,7 @@ private XFieldComparatorSource comparatorSource( source = new IntValuesComparatorSource(this, missingValue, sortMode, nested); } if (targetNumericType != getNumericType()) { - source.disableSkipping(); // disable skipping logic for caste of sort field + source.disableSkipping(); // disable skipping logic for cast of sort field } return source; } diff --git a/server/src/test/java/org/opensearch/index/IndexServiceTests.java b/server/src/test/java/org/opensearch/index/IndexServiceTests.java index a9fc108f6addb..db9f4bd305c79 100644 --- a/server/src/test/java/org/opensearch/index/IndexServiceTests.java +++ b/server/src/test/java/org/opensearch/index/IndexServiceTests.java @@ -543,6 +543,14 @@ public void testIndexSort() { index = createIndex("test", settings, createTestMapping("long")); assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.LONG); + // Float index sort should be remained to float sort type + index = createIndex("test", settings, createTestMapping("float")); + assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.FLOAT); + + // Double index sort should be remained to double sort type + index = createIndex("test", settings, createTestMapping("double")); + assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.DOUBLE); + // String index sort should be remained to string sort type index = createIndex("test", settings, createTestMapping("string")); assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.STRING); @@ -566,6 +574,14 @@ public void testIndexSortBackwardCompatible() { index = createIndex("test", settings, createTestMapping("long")); assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.LONG); + // Float index sort should be remained to float sort type + index = createIndex("test", settings, createTestMapping("float")); + assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.FLOAT); + + // Double index sort should be remained to double sort type + index = createIndex("test", settings, createTestMapping("double")); + assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.DOUBLE); + // String index sort should be remained to string sort type index = createIndex("test", settings, createTestMapping("string")); assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.STRING); From af7e9e9023cd4a6007bed3aeef1dc7c2fb275b92 Mon Sep 17 00:00:00 2001 From: gashutos Date: Fri, 15 Sep 2023 21:19:00 +0530 Subject: [PATCH 13/13] Removing unwanted imports Signed-off-by: gashutos --- .../src/test/java/org/opensearch/backwards/IndexingIT.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java b/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java index 77f4102a95cb9..13c2daeec37af 100644 --- a/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java +++ b/qa/mixed-cluster/src/test/java/org/opensearch/backwards/IndexingIT.java @@ -46,7 +46,6 @@ import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.common.xcontent.support.XContentMapValues; import org.opensearch.core.common.Strings; -import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.index.seqno.SeqNoStats; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.core.rest.RestStatus; @@ -64,7 +63,6 @@ import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; -import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; public class IndexingIT extends OpenSearchRestTestCase {