From 219f48d04cd1508209998dd5ca0f828cb122dc76 Mon Sep 17 00:00:00 2001 From: panguixin Date: Wed, 18 Dec 2024 23:40:44 +0800 Subject: [PATCH 1/2] use the correct type to widen the sort fields when merging top docs Signed-off-by: panguixin --- .../opensearch/search/sort/FieldSortIT.java | 40 ++++++++++++++ .../action/search/SearchPhaseController.java | 55 ++++++++++++------- .../sort/SortedWiderNumericSortField.java | 10 +++- 3 files changed, 84 insertions(+), 21 deletions(-) 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 fdb12639c65be..b0c53a4beba40 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java @@ -49,6 +49,7 @@ import org.opensearch.common.Numbers; import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.core.xcontent.XContentBuilder; @@ -2609,4 +2610,43 @@ public void testSimpleSortsPoints() throws Exception { assertThat(searchResponse.toString(), not(containsString("error"))); } + + public void testSortMixedNumericFields() throws Exception { + internalCluster().ensureAtLeastNumDataNodes(3); + index("long", Long.MAX_VALUE); + index("integer", Integer.MAX_VALUE); + SearchResponse searchResponse = client().prepareSearch("long", "integer") + .setQuery(matchAllQuery()) + .setSize(10) + .addSort(SortBuilders.fieldSort("field").order(SortOrder.ASC).sortMode(SortMode.MAX)) + .get(); + assertNoFailures(searchResponse); + long[] sortValues = new long[10]; + for (int i = 0; i < 10; i++) { + sortValues[i] = ((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).longValue(); + } + for (int i = 1; i < 10; i++) { + assertThat(Arrays.toString(sortValues), sortValues[i - 1], lessThan(sortValues[i])); + } + } + + private void index(String type, long end) throws Exception { + assertAcked( + prepareCreate(type).setMapping( + XContentFactory.jsonBuilder() + .startObject() + .startObject("properties") + .startObject("field") + .field("type", type) + .endObject() + .endObject() + .endObject() + ).setSettings(Settings.builder().put("index.number_of_shards", 3).put("index.number_of_replicas", 0)) + ); + ensureGreen(type); + for (int i = 0; i < 5; i++) { + client().prepareIndex(type).setId(Integer.toString(i)).setSource("{\"field\" : " + (end - i) + " }", XContentType.JSON).get(); + } + client().admin().indices().prepareRefresh(type).get(); + } } diff --git a/server/src/main/java/org/opensearch/action/search/SearchPhaseController.java b/server/src/main/java/org/opensearch/action/search/SearchPhaseController.java index 161a103cdf36a..84f2578eb6e2b 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchPhaseController.java +++ b/server/src/main/java/org/opensearch/action/search/SearchPhaseController.java @@ -48,6 +48,7 @@ import org.opensearch.common.lucene.search.TopDocsAndMaxScore; import org.opensearch.core.common.breaker.CircuitBreaker; import org.opensearch.core.common.io.stream.NamedWriteableRegistry; +import org.opensearch.index.fielddata.IndexFieldData; import org.opensearch.search.DocValueFormat; import org.opensearch.search.SearchHit; import org.opensearch.search.SearchHits; @@ -604,36 +605,52 @@ private static void validateMergeSortValueFormats(Collection comparator; + /** * Creates a sort, possibly in reverse, specifying how the sort value from the document's set is * selected. @@ -39,6 +43,8 @@ public class SortedWiderNumericSortField extends SortedNumericSortField { */ public SortedWiderNumericSortField(String field, Type type, boolean reverse) { super(field, type, reverse); + byteCounts = type == Type.LONG ? Long.BYTES : Double.BYTES; + comparator = type == Type.LONG ? Comparator.comparingLong(Number::longValue) : Comparator.comparingDouble(Number::doubleValue); } /** @@ -51,7 +57,7 @@ public SortedWiderNumericSortField(String field, Type type, boolean reverse) { */ @Override public FieldComparator getComparator(int numHits, Pruning pruning) { - return new NumericComparator(getField(), (Number) getMissingValue(), getReverse(), pruning, Double.BYTES) { + return new NumericComparator(getField(), (Number) getMissingValue(), getReverse(), pruning, byteCounts) { @Override public int compare(int slot1, int slot2) { throw new UnsupportedOperationException(); @@ -78,7 +84,7 @@ public int compareValues(Number first, Number second) { } else if (second == null) { return 1; } else { - return Double.compare(first.doubleValue(), second.doubleValue()); + return comparator.compare(first, second); } } }; From c011176a5883ff145ab51a11c3e610345ec15868 Mon Sep 17 00:00:00 2001 From: panguixin Date: Thu, 19 Dec 2024 13:35:58 +0800 Subject: [PATCH 2/2] fix Signed-off-by: panguixin --- .../org/opensearch/action/search/SearchPhaseController.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/search/SearchPhaseController.java b/server/src/main/java/org/opensearch/action/search/SearchPhaseController.java index 84f2578eb6e2b..9b6f904c99c4c 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchPhaseController.java +++ b/server/src/main/java/org/opensearch/action/search/SearchPhaseController.java @@ -623,11 +623,12 @@ private static Sort createSort(TopFieldDocs[] topFieldDocs) { SortField.Type sortType = getSortType(sortField); if (SortedWiderNumericSortField.isTypeSupported(sortType) == false) { // throw exception if sortType is not CUSTOM? + // skip this shard or do not widen? requireWiden = false; break; } - requireWiden = sortType != firstType; - isFloat = sortType == SortField.Type.FLOAT || sortType == SortField.Type.DOUBLE; + requireWiden = requireWiden || sortType != firstType; + isFloat = isFloat || sortType == SortField.Type.FLOAT || sortType == SortField.Type.DOUBLE; } if (requireWiden) {