Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Match phrase queries against non-indexed fields should throw an exception #31060

Merged
merged 7 commits into from
Jun 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/reference/migration/migrate_7_0/search.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 17 additions & 14 deletions server/src/main/java/org/elasticsearch/index/search/MatchQuery.java
Original file line number Diff line number Diff line change
Expand Up @@ -352,38 +352,41 @@ protected Query newSynonymQuery(Term[] terms) {

@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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ protected MatchPhrasePrefixQueryBuilder doCreateTestQueryBuilder() {

MatchPhrasePrefixQueryBuilder matchQuery = new MatchPhrasePrefixQueryBuilder(fieldName, value);

if (randomBoolean()) {
if (randomBoolean() && fieldName.equals(STRING_FIELD_NAME)) {
matchQuery.analyzer(randomFrom("simple", "keyword", "whitespace"));
}

Expand Down Expand Up @@ -99,15 +99,6 @@ protected void doAssertLuceneQuery(MatchPhrasePrefixQueryBuilder queryBuilder, Q
.or(instanceOf(IndexOrDocValuesQuery.class)).or(instanceOf(MatchNoDocsQuery.class)));
}

/**
* Overridden to allow for annotating with @AwaitsFix. Please remove this method after fixing.
*/
@Override
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/31061")
public void testToQuery() throws IOException {
super.testToQuery();
}

public void testIllegalValues() {
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new MatchPhrasePrefixQueryBuilder(null, "value"));
assertEquals("[match_phrase_prefix] requires fieldName", e.getMessage());
Expand All @@ -127,6 +118,12 @@ public void testBadAnalyzer() throws IOException {
assertThat(e.getMessage(), containsString("analyzer [bogusAnalyzer] not found"));
}

public void testPhraseOnFieldWithNoTerms() {
MatchPhrasePrefixQueryBuilder matchQuery = new MatchPhrasePrefixQueryBuilder(DATE_FIELD_NAME, "three term phrase");
matchQuery.analyzer("whitespace");
expectThrows(IllegalArgumentException.class, () -> matchQuery.doToQuery(createShardContext()));
}

public void testPhrasePrefixMatchQuery() throws IOException {
String json1 = "{\n" +
" \"match_phrase_prefix\" : {\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ protected MatchPhraseQueryBuilder doCreateTestQueryBuilder() {

MatchPhraseQueryBuilder matchQuery = new MatchPhraseQueryBuilder(fieldName, value);

if (randomBoolean()) {
if (randomBoolean() && fieldName.equals(STRING_FIELD_NAME)) {
matchQuery.analyzer(randomFrom("simple", "keyword", "whitespace"));
}

Expand Down Expand Up @@ -107,15 +107,6 @@ protected void doAssertLuceneQuery(MatchPhraseQueryBuilder queryBuilder, Query q
.or(instanceOf(IndexOrDocValuesQuery.class)).or(instanceOf(MatchNoDocsQuery.class)));
}

/**
* Overridden to allow for annotating with @AwaitsFix. Please remove this method after fixing.
*/
@Override
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/31061")
public void testToQuery() throws IOException {
super.testToQuery();
}

public void testIllegalValues() {
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new MatchPhraseQueryBuilder(null, "value"));
assertEquals("[match_phrase] requires fieldName", e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down