From 7388cabad318a52d663364a130676ca94dd245e1 Mon Sep 17 00:00:00 2001 From: Dimitris Rempapis Date: Thu, 19 Jun 2025 18:33:33 +0300 Subject: [PATCH 1/3] Add fix and test --- .../search/sort/RangeFieldSortIT.java | 101 ++++++++++++++++++ .../search/sort/FieldSortBuilder.java | 6 ++ 2 files changed, 107 insertions(+) create mode 100644 server/src/internalClusterTest/java/org/elasticsearch/search/sort/RangeFieldSortIT.java diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/sort/RangeFieldSortIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/sort/RangeFieldSortIT.java new file mode 100644 index 0000000000000..6187dbc5f4f85 --- /dev/null +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/sort/RangeFieldSortIT.java @@ -0,0 +1,101 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.search.sort; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.mapper.RangeType; +import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.test.ESSingleNodeTestCase; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentFactory; + +import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFailures; + +public class RangeFieldSortIT extends ESSingleNodeTestCase { + + private static final String FIELD_NAME = "range"; + + public void testSortingOnIntegerRangeFieldThrows400() throws Exception { + String indexName = "int_range_index"; + ; + createIndex(indexName, FIELD_NAME, RangeType.INTEGER.typeName()); + assertFailures( + client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), + RestStatus.BAD_REQUEST + ); + } + + public void testSortingOnLongRangeFieldThrows400() throws Exception { + String indexName = "long_range_index"; + createIndex(indexName, FIELD_NAME, RangeType.LONG.typeName()); + assertFailures( + client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), + RestStatus.BAD_REQUEST + ); + } + + public void testSortingOnFloatRangeFieldThrows400() throws Exception { + String indexName = "float_range_index"; + createIndex(indexName, FIELD_NAME, RangeType.FLOAT.typeName()); + assertFailures( + client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), + RestStatus.BAD_REQUEST + ); + } + + public void testSortingOnDoubleRangeFieldThrows400() throws Exception { + String indexName = "double_range_index"; + createIndex(indexName, FIELD_NAME, RangeType.DOUBLE.typeName()); + assertFailures( + client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), + RestStatus.BAD_REQUEST + ); + } + + public void testSortingOnIpRangeFieldThrows400() throws Exception { + String indexName = "ip_range_index"; + createIndex(indexName, FIELD_NAME, RangeType.IP.typeName()); + assertFailures( + client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), + RestStatus.BAD_REQUEST + ); + } + + public void testSortingOnDateRangeFieldThrows400() throws Exception { + String indexName = "date_range_index"; + createIndex(indexName, FIELD_NAME, RangeType.DATE.typeName()); + assertFailures( + client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), + RestStatus.BAD_REQUEST + ); + } + + private void createIndex(String indexName, String rangeFieldName, String rangeFieldType) throws Exception { + int numShards = randomIntBetween(1, 3); + client().admin() + .indices() + .prepareCreate(indexName) + .setSettings(Settings.builder().put("index.number_of_shards", numShards)) + .setMapping(createMapping(rangeFieldName, rangeFieldType)) + .get(); + } + + private XContentBuilder createMapping(String fieldName, String fieldType) throws Exception { + return XContentFactory.jsonBuilder() + .startObject() + .startObject("properties") + .startObject(fieldName) + .field("type", fieldType) + .endObject() + .endObject() + .endObject(); + } +} diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index a720d2e4759d4..0ad20f0f8b882 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -34,6 +34,7 @@ import org.elasticsearch.index.mapper.NestedLookup; import org.elasticsearch.index.mapper.NestedObjectMapper; import org.elasticsearch.index.mapper.NumberFieldMapper.NumberFieldType; +import org.elasticsearch.index.mapper.RangeFieldMapper; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.QueryShardException; @@ -347,6 +348,11 @@ public SortFieldAndFormat build(SearchExecutionContext context) throws IOExcepti } MappedFieldType fieldType = context.getFieldType(fieldName); + + if (fieldType instanceof RangeFieldMapper.RangeFieldType) { + throw new IllegalArgumentException("Sorting by range field [" + fieldName + "] is not supported"); + } + Nested nested = nested(context, fieldType); if (fieldType == null) { fieldType = resolveUnmappedType(context); From e0ef65beebd0f282ed1b23ecdb80189335772853 Mon Sep 17 00:00:00 2001 From: Dimitris Rempapis Date: Thu, 19 Jun 2025 18:38:24 +0300 Subject: [PATCH 2/3] Update docs/changelog/129725.yaml --- docs/changelog/129725.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/129725.yaml diff --git a/docs/changelog/129725.yaml b/docs/changelog/129725.yaml new file mode 100644 index 0000000000000..8c14844871297 --- /dev/null +++ b/docs/changelog/129725.yaml @@ -0,0 +1,5 @@ +pr: 129725 +summary: Throw a 400 when sorting for all types of range fields +area: Search +type: bug +issues: [] From 0a7d147b1240306b4292fec937cf9b1ee41325da Mon Sep 17 00:00:00 2001 From: Dimitris Rempapis Date: Thu, 26 Jun 2025 15:16:41 +0300 Subject: [PATCH 3/3] update after review --- .../search/sort/RangeFieldSortIT.java | 20 +++++++++++------ .../index/mapper/RangeFieldMapper.java | 22 ++++++++++++++++++- .../search/sort/FieldSortBuilder.java | 6 ----- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/sort/RangeFieldSortIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/sort/RangeFieldSortIT.java index 6187dbc5f4f85..8f3dea78b73e9 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/sort/RangeFieldSortIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/sort/RangeFieldSortIT.java @@ -18,6 +18,7 @@ import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFailures; +import static org.hamcrest.Matchers.containsString; public class RangeFieldSortIT extends ESSingleNodeTestCase { @@ -25,11 +26,11 @@ public class RangeFieldSortIT extends ESSingleNodeTestCase { public void testSortingOnIntegerRangeFieldThrows400() throws Exception { String indexName = "int_range_index"; - ; createIndex(indexName, FIELD_NAME, RangeType.INTEGER.typeName()); assertFailures( client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), - RestStatus.BAD_REQUEST + RestStatus.BAD_REQUEST, + containsString("Sorting by range field [" + FIELD_NAME + "] is not supported") ); } @@ -38,7 +39,8 @@ public void testSortingOnLongRangeFieldThrows400() throws Exception { createIndex(indexName, FIELD_NAME, RangeType.LONG.typeName()); assertFailures( client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), - RestStatus.BAD_REQUEST + RestStatus.BAD_REQUEST, + containsString("Sorting by range field [" + FIELD_NAME + "] is not supported") ); } @@ -47,7 +49,8 @@ public void testSortingOnFloatRangeFieldThrows400() throws Exception { createIndex(indexName, FIELD_NAME, RangeType.FLOAT.typeName()); assertFailures( client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), - RestStatus.BAD_REQUEST + RestStatus.BAD_REQUEST, + containsString("Sorting by range field [" + FIELD_NAME + "] is not supported") ); } @@ -56,7 +59,8 @@ public void testSortingOnDoubleRangeFieldThrows400() throws Exception { createIndex(indexName, FIELD_NAME, RangeType.DOUBLE.typeName()); assertFailures( client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), - RestStatus.BAD_REQUEST + RestStatus.BAD_REQUEST, + containsString("Sorting by range field [" + FIELD_NAME + "] is not supported") ); } @@ -65,7 +69,8 @@ public void testSortingOnIpRangeFieldThrows400() throws Exception { createIndex(indexName, FIELD_NAME, RangeType.IP.typeName()); assertFailures( client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), - RestStatus.BAD_REQUEST + RestStatus.BAD_REQUEST, + containsString("Sorting by range field [" + FIELD_NAME + "] is not supported") ); } @@ -74,7 +79,8 @@ public void testSortingOnDateRangeFieldThrows400() throws Exception { createIndex(indexName, FIELD_NAME, RangeType.DATE.typeName()); assertFailures( client().prepareSearch(indexName).setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort(FIELD_NAME).order(SortOrder.ASC)), - RestStatus.BAD_REQUEST + RestStatus.BAD_REQUEST, + containsString("Sorting by range field [" + FIELD_NAME + "] is not supported") ); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java index 79a0e6b1fdbc4..e53fc3cba5d58 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java @@ -10,6 +10,7 @@ package org.elasticsearch.index.mapper; import org.apache.lucene.search.Query; +import org.apache.lucene.search.SortField; import org.apache.lucene.util.BytesRef; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.Explicit; @@ -20,13 +21,17 @@ import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.time.DateMathParser; import org.elasticsearch.common.util.LocaleUtils; +import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Tuple; import org.elasticsearch.features.NodeFeature; import org.elasticsearch.index.fielddata.FieldDataContext; import org.elasticsearch.index.fielddata.IndexFieldData; +import org.elasticsearch.index.fielddata.IndexFieldDataCache; import org.elasticsearch.index.fielddata.plain.BinaryIndexFieldData; import org.elasticsearch.index.query.SearchExecutionContext; +import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.search.DocValueFormat; +import org.elasticsearch.search.MultiValueMode; import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentParser; @@ -227,7 +232,22 @@ public RangeType rangeType() { @Override public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext) { failIfNoDocValues(); - return new BinaryIndexFieldData.Builder(name(), CoreValuesSourceType.RANGE); + return new BinaryIndexFieldData.Builder(name(), CoreValuesSourceType.RANGE) { + @Override + public BinaryIndexFieldData build(IndexFieldDataCache cache, CircuitBreakerService breakerService) { + return new BinaryIndexFieldData(name(), CoreValuesSourceType.RANGE) { + @Override + public SortField sortField( + @Nullable Object missingValue, + MultiValueMode sortMode, + XFieldComparatorSource.Nested nested, + boolean reverse + ) { + throw new IllegalArgumentException("Sorting by range field [" + name() + "] is not supported"); + } + }; + } + }; } @Override diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index 0ad20f0f8b882..a720d2e4759d4 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -34,7 +34,6 @@ import org.elasticsearch.index.mapper.NestedLookup; import org.elasticsearch.index.mapper.NestedObjectMapper; import org.elasticsearch.index.mapper.NumberFieldMapper.NumberFieldType; -import org.elasticsearch.index.mapper.RangeFieldMapper; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.QueryShardException; @@ -348,11 +347,6 @@ public SortFieldAndFormat build(SearchExecutionContext context) throws IOExcepti } MappedFieldType fieldType = context.getFieldType(fieldName); - - if (fieldType instanceof RangeFieldMapper.RangeFieldType) { - throw new IllegalArgumentException("Sorting by range field [" + fieldName + "] is not supported"); - } - Nested nested = nested(context, fieldType); if (fieldType == null) { fieldType = resolveUnmappedType(context);