From 852df128a5544dbafaf7f901e31f3d88431c767e Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 4 Jun 2018 19:12:45 +0100 Subject: [PATCH] Match phrase queries against non-indexed fields should throw an exception (#31060) When `lenient=false`, attempts to create match phrase queries with custom analyzers against non-text fields will throw an IllegalArgumentException. Also changes `*Match*QueryBuilderTests` so that it avoids this scenario Fixes #31061 --- .../migration/migrate_7_0/search.asciidoc | 3 ++ .../index/search/MatchQuery.java | 31 ++++++++++--------- .../MatchPhrasePrefixQueryBuilderTests.java | 17 +++++----- .../query/MatchPhraseQueryBuilderTests.java | 11 +------ .../query/MultiMatchQueryBuilderTests.java | 1 + 5 files changed, 29 insertions(+), 34 deletions(-) diff --git a/docs/reference/migration/migrate_7_0/search.asciidoc b/docs/reference/migration/migrate_7_0/search.asciidoc index 7505b6f14d1..53a7d88394b 100644 --- a/docs/reference/migration/migrate_7_0/search.asciidoc +++ b/docs/reference/migration/migrate_7_0/search.asciidoc @@ -15,6 +15,9 @@ * The boundary specified using geohashes in the `geo_bounding_box` query now include entire geohash cell, instead of just geohash center. +* Attempts to generate multi-term phrase queries against non-text fields + with a custom analyzer will now throw an exception + ==== Adaptive replica selection enabled by default Adaptive replica selection has been enabled by default. If you wish to return to diff --git a/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java b/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java index 43c842a1697..d60d9cd9ce6 100644 --- a/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java +++ b/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java @@ -352,38 +352,41 @@ public class MatchQuery { @Override protected Query analyzePhrase(String field, TokenStream stream, int slop) throws IOException { - IllegalStateException e = checkForPositions(field); - if (e != null) { + try { + checkForPositions(field); + Query query = mapper.phraseQuery(field, stream, slop, enablePositionIncrements); + if (query instanceof PhraseQuery) { + // synonyms that expand to multiple terms can return a phrase query. + return blendPhraseQuery((PhraseQuery) query, mapper); + } + return query; + } + catch (IllegalArgumentException | IllegalStateException e) { if (lenient) { return newLenientFieldQuery(field, e); } throw e; } - Query query = mapper.phraseQuery(field, stream, slop, enablePositionIncrements); - if (query instanceof PhraseQuery) { - // synonyms that expand to multiple terms can return a phrase query. - return blendPhraseQuery((PhraseQuery) query, mapper); - } - return query; } @Override protected Query analyzeMultiPhrase(String field, TokenStream stream, int slop) throws IOException { - IllegalStateException e = checkForPositions(field); - if (e != null) { + try { + checkForPositions(field); + return mapper.multiPhraseQuery(field, stream, slop, enablePositionIncrements); + } + catch (IllegalArgumentException | IllegalStateException e) { if (lenient) { return newLenientFieldQuery(field, e); } throw e; } - return mapper.multiPhraseQuery(field, stream, slop, enablePositionIncrements); } - private IllegalStateException checkForPositions(String field) { + private void checkForPositions(String field) { if (hasPositions(mapper) == false) { - return new IllegalStateException("field:[" + field + "] was indexed without position data; cannot run PhraseQuery"); + throw new IllegalStateException("field:[" + field + "] was indexed without position data; cannot run PhraseQuery"); } - return null; } /** diff --git a/server/src/test/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilderTests.java index 00701adc449..8b706e552bc 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilderTests.java @@ -61,7 +61,7 @@ public class MatchPhrasePrefixQueryBuilderTests extends AbstractQueryTestCase new MatchPhrasePrefixQueryBuilder(null, "value")); assertEquals("[match_phrase_prefix] requires fieldName", e.getMessage()); @@ -127,6 +118,12 @@ public class MatchPhrasePrefixQueryBuilderTests extends AbstractQueryTestCase matchQuery.doToQuery(createShardContext())); + } + public void testPhrasePrefixMatchQuery() throws IOException { String json1 = "{\n" + " \"match_phrase_prefix\" : {\n" + diff --git a/server/src/test/java/org/elasticsearch/index/query/MatchPhraseQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MatchPhraseQueryBuilderTests.java index 91a775dbf02..fb03d54aeff 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MatchPhraseQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MatchPhraseQueryBuilderTests.java @@ -64,7 +64,7 @@ public class MatchPhraseQueryBuilderTests extends AbstractQueryTestCase new MatchPhraseQueryBuilder(null, "value")); assertEquals("[match_phrase] requires fieldName", e.getMessage()); diff --git a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java index b6d4816b01f..fcdf128c416 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.query; +import com.carrotsearch.randomizedtesting.annotations.Seed; import org.apache.lucene.index.Term; import org.apache.lucene.queries.ExtendedCommonTermsQuery; import org.apache.lucene.search.BooleanQuery;