From a5fb0e8123a5adc6176b6aa703039edce2a85cf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 3 Jul 2018 12:10:30 +0200 Subject: [PATCH 1/3] Fix `range` queries on `_type` field for singe type indices With the introduction of single types in 6.x, the `_type` field is no longer indexed, which leads to certain queries that were working before throw errors now. One such query is the `range` query, that, if performed on a single typer index, currently throws an IAE since the field is not indexed. This change adds special treatment for this case in the TypeFieldMapper, comparing the range queries lower and upper bound to the one existing type and either returns a MatchAllDocs or a MatchNoDocs query. Relates to #31632 Closes #31476 --- .../index/mapper/TypeFieldMapper.java | 32 +++++++++++++++++ .../search/query/SearchQueryIT.java | 36 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java index ffb548fd0f10f..2ed7094193ae1 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java @@ -34,7 +34,10 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.TermInSetQuery; import org.apache.lucene.search.TermQuery; +import org.apache.lucene.search.TermRangeQuery; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -90,6 +93,8 @@ public MetadataFieldMapper getDefault(MappedFieldType fieldType, ParserContext c static final class TypeFieldType extends StringFieldType { + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(ESLoggerFactory.getLogger(TypeFieldType.class)); + TypeFieldType() { } @@ -154,6 +159,33 @@ public Query termsQuery(List values, QueryShardContext context) { } } + @Override + public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, QueryShardContext context) { + if (hasDocValues()) { + return new TermRangeQuery(name(), lowerTerm == null ? null : indexedValueForSearch(lowerTerm), + upperTerm == null ? null : indexedValueForSearch(upperTerm), includeLower, includeUpper); + } else { + // this means the index has a single type and the type field is implicit + DEPRECATION_LOGGER.deprecatedAndMaybeLog("range_single_type", + "Running [range] query on [_type] field for an index with a single type. As types are deprecated, this functionality will be removed in future releases."); + String type = context.getMapperService().documentMapper().type(); + Query result = new MatchAllDocsQuery(); + if (lowerTerm != null && lowerTerm instanceof BytesRef) { + int comp = type.compareTo(((BytesRef) lowerTerm).utf8ToString()); + if (comp < 0 || (comp == 0 && includeLower == false)) { + result = new MatchNoDocsQuery("[_type] was less then lower bound of range"); + } + } + if (upperTerm != null && upperTerm instanceof BytesRef) { + int comp = type.compareTo(((BytesRef) upperTerm).utf8ToString()); + if (comp > 0 || (comp == 0 && includeUpper == false)) { + result = new MatchNoDocsQuery("[_type] was higher then upper bound of range"); + } + } + return result; + } + } + } /** diff --git a/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java b/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java index eab3a6e9b4824..ee3cbd9e9917d 100644 --- a/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java @@ -1818,4 +1818,40 @@ public void testRangeQueryRangeFields_24744() throws Exception { SearchResponse searchResponse = client().prepareSearch("test").setQuery(range).get(); assertHitCount(searchResponse, 1); } + + public void testRangeQueryTypeField_31476() throws Exception { + assertAcked(prepareCreate("test").addMapping("foo", "field", "type=keyword")); + + client().prepareIndex("test", "foo", "1").setSource("field", "value").get(); + refresh(); + + RangeQueryBuilder range = new RangeQueryBuilder("_type").from("ape").to("zebra"); + SearchResponse searchResponse = client().prepareSearch("test").setQuery(range).get(); + assertHitCount(searchResponse, 1); + + range = new RangeQueryBuilder("_type").from("monkey").to("zebra"); + searchResponse = client().prepareSearch("test").setQuery(range).get(); + assertHitCount(searchResponse, 0); + + range = new RangeQueryBuilder("_type").from("ape").to("donkey"); + searchResponse = client().prepareSearch("test").setQuery(range).get(); + assertHitCount(searchResponse, 0); + + range = new RangeQueryBuilder("_type").from("ape").to("foo").includeUpper(false); + searchResponse = client().prepareSearch("test").setQuery(range).get(); + assertHitCount(searchResponse, 0); + + range = new RangeQueryBuilder("_type").from("ape").to("foo").includeUpper(true); + searchResponse = client().prepareSearch("test").setQuery(range).get(); + assertHitCount(searchResponse, 1); + + range = new RangeQueryBuilder("_type").from("foo").to("zebra").includeLower(false); + searchResponse = client().prepareSearch("test").setQuery(range).get(); + assertHitCount(searchResponse, 0); + + range = new RangeQueryBuilder("_type").from("foo").to("zebra").includeLower(true); + searchResponse = client().prepareSearch("test").setQuery(range).get(); + assertHitCount(searchResponse, 1); + } + } From 04b2d9d8afc1e82a1de78006fe8b882cef3696e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 3 Jul 2018 21:25:29 +0200 Subject: [PATCH 2/3] iter --- .../index/mapper/TypeFieldMapper.java | 38 ++++++++++--------- .../search/query/SearchQueryIT.java | 8 ++++ 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java index 2ed7094193ae1..fee6743f6369f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java @@ -34,7 +34,6 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.TermInSetQuery; import org.apache.lucene.search.TermQuery; -import org.apache.lucene.search.TermRangeQuery; import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.ESLoggerFactory; @@ -161,31 +160,34 @@ public Query termsQuery(List values, QueryShardContext context) { @Override public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, QueryShardContext context) { - if (hasDocValues()) { - return new TermRangeQuery(name(), lowerTerm == null ? null : indexedValueForSearch(lowerTerm), - upperTerm == null ? null : indexedValueForSearch(upperTerm), includeLower, includeUpper); - } else { - // this means the index has a single type and the type field is implicit - DEPRECATION_LOGGER.deprecatedAndMaybeLog("range_single_type", - "Running [range] query on [_type] field for an index with a single type. As types are deprecated, this functionality will be removed in future releases."); - String type = context.getMapperService().documentMapper().type(); - Query result = new MatchAllDocsQuery(); - if (lowerTerm != null && lowerTerm instanceof BytesRef) { - int comp = type.compareTo(((BytesRef) lowerTerm).utf8ToString()); - if (comp < 0 || (comp == 0 && includeLower == false)) { + DEPRECATION_LOGGER.deprecatedAndMaybeLog("range_single_type", + "Running [range] query on [_type] field for an index with a single type. As types are deprecated, this functionality will be removed in future releases."); + if ((lowerTerm != null && lowerTerm instanceof BytesRef == false)) { + throw new IllegalArgumentException("lower term should be BytesRef but was: " + lowerTerm.getClass().getName()); + } + if ((upperTerm != null && upperTerm instanceof BytesRef == false)) { + throw new IllegalArgumentException("upper term should be BytesRef but was: " + upperTerm.getClass().getName()); + } + + Query result = new MatchAllDocsQuery(); + String type = context.getMapperService().documentMapper().type(); + if (type != null) { + BytesRef typeBytes = new BytesRef(type); + if (lowerTerm != null) { + int comp = ((BytesRef) lowerTerm).compareTo(typeBytes); + if (comp > 0 || (comp == 0 && includeLower == false)) { result = new MatchNoDocsQuery("[_type] was less then lower bound of range"); } } - if (upperTerm != null && upperTerm instanceof BytesRef) { - int comp = type.compareTo(((BytesRef) upperTerm).utf8ToString()); - if (comp > 0 || (comp == 0 && includeUpper == false)) { + if (upperTerm != null) { + int comp = ((BytesRef) upperTerm).compareTo(typeBytes); + if (comp < 0 || (comp == 0 && includeUpper == false)) { result = new MatchNoDocsQuery("[_type] was higher then upper bound of range"); } } - return result; } + return result; } - } /** diff --git a/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java b/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java index ee3cbd9e9917d..ea221b29f9f77 100644 --- a/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java @@ -1852,6 +1852,14 @@ public void testRangeQueryTypeField_31476() throws Exception { range = new RangeQueryBuilder("_type").from("foo").to("zebra").includeLower(true); searchResponse = client().prepareSearch("test").setQuery(range).get(); assertHitCount(searchResponse, 1); + + Exception ex = expectThrows(Exception.class, () -> client().prepareSearch("test") + .setQuery(new RangeQueryBuilder("_type").from(123L).to("zebra").includeLower(true)).get()); + assertEquals("lower term should be BytesRef but was: java.lang.Long", ex.getCause().getCause().getMessage()); + + ex = expectThrows(Exception.class, () -> client().prepareSearch("test") + .setQuery(new RangeQueryBuilder("_type").from("ape").to(true).includeLower(true)).get()); + assertEquals("upper term should be BytesRef but was: java.lang.Boolean", ex.getCause().getCause().getMessage()); } } From c5f698ab641c0f8d1ff06aa94eb642d482fe939b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 17 Jul 2018 16:00:50 +0200 Subject: [PATCH 3/3] iter --- .../index/mapper/TypeFieldMapper.java | 15 ++++----------- .../elasticsearch/search/query/SearchQueryIT.java | 8 -------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java index fee6743f6369f..71bd2e93d3039 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java @@ -162,27 +162,20 @@ public Query termsQuery(List values, QueryShardContext context) { public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, QueryShardContext context) { DEPRECATION_LOGGER.deprecatedAndMaybeLog("range_single_type", "Running [range] query on [_type] field for an index with a single type. As types are deprecated, this functionality will be removed in future releases."); - if ((lowerTerm != null && lowerTerm instanceof BytesRef == false)) { - throw new IllegalArgumentException("lower term should be BytesRef but was: " + lowerTerm.getClass().getName()); - } - if ((upperTerm != null && upperTerm instanceof BytesRef == false)) { - throw new IllegalArgumentException("upper term should be BytesRef but was: " + upperTerm.getClass().getName()); - } - Query result = new MatchAllDocsQuery(); String type = context.getMapperService().documentMapper().type(); if (type != null) { BytesRef typeBytes = new BytesRef(type); if (lowerTerm != null) { - int comp = ((BytesRef) lowerTerm).compareTo(typeBytes); + int comp = indexedValueForSearch(lowerTerm).compareTo(typeBytes); if (comp > 0 || (comp == 0 && includeLower == false)) { - result = new MatchNoDocsQuery("[_type] was less then lower bound of range"); + result = new MatchNoDocsQuery("[_type] was lexicographically smaller than lower bound of range"); } } if (upperTerm != null) { - int comp = ((BytesRef) upperTerm).compareTo(typeBytes); + int comp = indexedValueForSearch(upperTerm).compareTo(typeBytes); if (comp < 0 || (comp == 0 && includeUpper == false)) { - result = new MatchNoDocsQuery("[_type] was higher then upper bound of range"); + result = new MatchNoDocsQuery("[_type] was lexicographically greater than upper bound of range"); } } } diff --git a/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java b/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java index bc8fc6334858d..1694f86c53eac 100644 --- a/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java @@ -1856,14 +1856,6 @@ public void testRangeQueryTypeField_31476() throws Exception { range = new RangeQueryBuilder("_type").from("foo").to("zebra").includeLower(true); searchResponse = client().prepareSearch("test").setQuery(range).get(); assertHitCount(searchResponse, 1); - - Exception ex = expectThrows(Exception.class, () -> client().prepareSearch("test") - .setQuery(new RangeQueryBuilder("_type").from(123L).to("zebra").includeLower(true)).get()); - assertEquals("lower term should be BytesRef but was: java.lang.Long", ex.getCause().getCause().getMessage()); - - ex = expectThrows(Exception.class, () -> client().prepareSearch("test") - .setQuery(new RangeQueryBuilder("_type").from("ape").to(true).includeLower(true)).get()); - assertEquals("upper term should be BytesRef but was: java.lang.Boolean", ex.getCause().getCause().getMessage()); } }