Skip to content

Commit d7a43b0

Browse files
author
Christoph Büscher
committed
Handle range query edge case
Currently when searching with an empty string as lower bound for a range query on text-based fields we return all documents when 'gte' is used (including the lower bound) but no documents when 'gt' is used. This might seem counterintuitive since every value should be greate than the empty string. This PR fixed this special edge case by implicitely setting the "lower" include flag in this case before constructing the TermRangeQuery. Closes elastic#63386
1 parent 9d5f59e commit d7a43b0

File tree

3 files changed

+52
-1
lines changed

3 files changed

+52
-1
lines changed

server/src/internalClusterTest/java/org/elasticsearch/search/simple/SimpleSearchIT.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,46 @@ public void testSimpleDateRange() throws Exception {
225225
assertHitCount(searchResponse, 2L);
226226
}
227227

228+
public void testRangeQueryKeyword() throws Exception {
229+
createIndex("test");
230+
231+
client().admin().indices().preparePutMapping("test").setSource("field", "type=keyword").get();
232+
233+
client().prepareIndex("test").setId("1").setSource("field", "A").get();
234+
client().prepareIndex("test").setId("2").setSource("field", "B").get();
235+
client().prepareIndex("test").setId("3").setSource("field", "C").get();
236+
ensureGreen();
237+
refresh();
238+
239+
SearchResponse searchResponse = client().prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gte("A").lte("B")).get();
240+
assertNoFailures(searchResponse);
241+
assertHitCount(searchResponse, 2L);
242+
243+
searchResponse = client().prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gt("A").lte("B")).get();
244+
assertNoFailures(searchResponse);
245+
assertHitCount(searchResponse, 1L);
246+
247+
searchResponse = client().prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gte("A").lt("B")).get();
248+
assertNoFailures(searchResponse);
249+
assertHitCount(searchResponse, 1L);
250+
251+
searchResponse = client().prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gte(null).lt("C")).get();
252+
assertNoFailures(searchResponse);
253+
assertHitCount(searchResponse, 2L);
254+
255+
searchResponse = client().prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gte("B").lt(null)).get();
256+
assertNoFailures(searchResponse);
257+
assertHitCount(searchResponse, 2L);
258+
259+
searchResponse = client().prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gt(null).lt(null)).get();
260+
assertNoFailures(searchResponse);
261+
assertHitCount(searchResponse, 3L);
262+
263+
searchResponse = client().prepareSearch("test").setQuery(QueryBuilders.rangeQuery("field").gt("").lt(null)).get();
264+
assertNoFailures(searchResponse);
265+
assertHitCount(searchResponse, 3L);
266+
}
267+
228268
public void testSimpleTerminateAfterCount() throws Exception {
229269
prepareCreate("test").setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0)).get();
230270
ensureGreen();

server/src/main/java/org/elasticsearch/index/mapper/StringFieldType.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,12 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
170170
ALLOW_EXPENSIVE_QUERIES.getKey() + "' is set to false.");
171171
}
172172
failIfNotIndexed();
173+
BytesRef indexedLower = lowerTerm == null ? null : indexedValueForSearch(lowerTerm);
174+
if (indexedLower != null && indexedLower.length == 0) {
175+
includeLower = true;
176+
}
173177
return new TermRangeQuery(name(),
174-
lowerTerm == null ? null : indexedValueForSearch(lowerTerm),
178+
indexedLower,
175179
upperTerm == null ? null : indexedValueForSearch(upperTerm),
176180
includeLower, includeUpper);
177181
}

server/src/test/java/org/elasticsearch/index/mapper/TextFieldTypeTests.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,13 @@ public void testRangeQuery() {
9595
assertEquals(new TermRangeQuery("field", BytesRefs.toBytesRef("foo"), BytesRefs.toBytesRef("bar"), true, false),
9696
ft.rangeQuery("foo", "bar", true, false, null, null, null, MOCK_QSC));
9797

98+
// empty string lower bound should always result query with lower-include flag being set
99+
assertEquals(new TermRangeQuery("field", BytesRefs.toBytesRef(""), null, true, true),
100+
ft.rangeQuery("", null, true, true, null, null, null, MOCK_QSC));
101+
102+
assertEquals(new TermRangeQuery("field", BytesRefs.toBytesRef(""), null, true, true),
103+
ft.rangeQuery("", null, false, true, null, null, null, MOCK_QSC));
104+
98105
ElasticsearchException ee = expectThrows(ElasticsearchException.class,
99106
() -> ft.rangeQuery("foo", "bar", true, false, null, null, null, MOCK_QSC_DISALLOW_EXPENSIVE));
100107
assertEquals("[range] queries on [text] or [keyword] fields cannot be executed when " +

0 commit comments

Comments
 (0)