honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-17 10:25:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Throw parsing error if match_phrase query contains multiple fields

Browse Source 
Match phrase Query, like many other queries, used to parse even when the query referred to multiple fields and the first one would win. We rather throw an exception now instead.
Also added test for short prefix query variant and modified the parsing code to consume the whole query object.
This commit is contained in:
javanna 2016-08-03 18:57:18 +02:00 committed by Luca Cavanna
parent 51ea913248
commit f56333048a
2 changed files with 78 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
core/src/main/java/org/elasticsearch/index/query/MatchPhraseQueryBuilder.java
View File

@ -22,6 +22,7 @@ package org.elasticsearch.index.query;
import org.apache.lucene.search.Query;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder;
@ -49,7 +50,7 @@ public class MatchPhraseQueryBuilder extends AbstractQueryBuilder<MatchPhraseQue
private int slop = MatchQuery.DEFAULT_PHRASE_SLOP;
public MatchPhraseQueryBuilder(String fieldName, Object value) {
if (fieldName == null) {
if (Strings.isEmpty(fieldName)) {
throw new IllegalArgumentException("[" + NAME + "] requires fieldName");
}
if (value == null) {
@ -163,59 +164,52 @@ public class MatchPhraseQueryBuilder extends AbstractQueryBuilder<MatchPhraseQue
public static Optional<MatchPhraseQueryBuilder> fromXContent(QueryParseContext parseContext) throws IOException {
XContentParser parser = parseContext.parser();
XContentParser.Token token = parser.nextToken();
if (token != XContentParser.Token.FIELD_NAME) {
throw new ParsingException(parser.getTokenLocation(), "[" + NAME + "] query malformed, no field");
}
String fieldName = parser.currentName();
String fieldName = null;
Object value = null;
float boost = AbstractQueryBuilder.DEFAULT_BOOST;
String analyzer = null;
int slop = MatchQuery.DEFAULT_PHRASE_SLOP;
String queryName = null;
token = parser.nextToken();
if (token == XContentParser.Token.START_OBJECT) {
String currentFieldName = null;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if (token.isValue()) {
if (parseContext.getParseFieldMatcher().match(currentFieldName, MatchQueryBuilder.QUERY_FIELD)) {
value = parser.objectText();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, MatchQueryBuilder.ANALYZER_FIELD)) {
analyzer = parser.text();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) {
boost = parser.floatValue();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, SLOP_FIELD)) {
slop = parser.intValue();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) {
queryName = parser.text();
String currentFieldName = null;
XContentParser.Token token;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if (parseContext.isDeprecatedSetting(currentFieldName)) {
// skip
} else if (token == XContentParser.Token.START_OBJECT) {
if (fieldName != null) {
throw new ParsingException(parser.getTokenLocation(), "[match_phrase] query doesn't support multiple fields, found ["
+ fieldName + "] and [" + currentFieldName + "]");
}
fieldName = currentFieldName;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if (token.isValue()) {
if (parseContext.getParseFieldMatcher().match(currentFieldName, MatchQueryBuilder.QUERY_FIELD)) {
value = parser.objectText();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, MatchQueryBuilder.ANALYZER_FIELD)) {
analyzer = parser.text();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) {
boost = parser.floatValue();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, SLOP_FIELD)) {
slop = parser.intValue();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) {
queryName = parser.text();
} else {
throw new ParsingException(parser.getTokenLocation(),
"[" + NAME + "] query does not support [" + currentFieldName + "]");
}
} else {
throw new ParsingException(parser.getTokenLocation(),
"[" + NAME + "] query does not support [" + currentFieldName + "]");
"[" + NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]");
}
} else {
throw new ParsingException(parser.getTokenLocation(),
"[" + NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]");
}
} else {
fieldName = parser.currentName();
value = parser.objectText();
}
parser.nextToken();
} else {
value = parser.objectText();
// move to the next token
token = parser.nextToken();
if (token != XContentParser.Token.END_OBJECT) {
throw new ParsingException(parser.getTokenLocation(), "[" + NAME
+ "] query parsed in simplified form, with direct field name, "
+ "but included more options than just the field name, possibly use its 'options' form, with 'query' element?");
}
}
if (value == null) {
throw new ParsingException(parser.getTokenLocation(), "No text specified for text query");
}
MatchPhraseQueryBuilder matchQuery = new MatchPhraseQueryBuilder(fieldName, value);

43
core/src/test/java/org/elasticsearch/index/query/MatchPhraseQueryBuilderTests.java
View File

@ -24,10 +24,13 @@ import org.apache.lucene.search.PhraseQuery;
import org.apache.lucene.search.PointRangeQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.lucene.search.MatchNoDocsQuery;
import org.elasticsearch.test.AbstractQueryTestCase;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import static org.hamcrest.CoreMatchers.either;
import static org.hamcrest.CoreMatchers.instanceOf;
@ -66,6 +69,20 @@ public class MatchPhraseQueryBuilderTests extends AbstractQueryTestCase<MatchPhr
return matchQuery;
}
@Override
protected Map<String, MatchPhraseQueryBuilder> getAlternateVersions() {
Map<String, MatchPhraseQueryBuilder> alternateVersions = new HashMap<>();
MatchPhraseQueryBuilder matchPhraseQuery = new MatchPhraseQueryBuilder(randomAsciiOfLengthBetween(1, 10),
randomAsciiOfLengthBetween(1, 10));
String contentString = "{\n" +
" \"match_phrase\" : {\n" +
" \"" + matchPhraseQuery.fieldName() + "\" : \"" + matchPhraseQuery.value() + "\"\n" +
" }\n" +
"}";
alternateVersions.put(contentString, matchPhraseQuery);
return alternateVersions;
}
@Override
protected void doAssertLuceneQuery(MatchPhraseQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
assertThat(query, notNullValue());
@ -76,16 +93,16 @@ public class MatchPhraseQueryBuilderTests extends AbstractQueryTestCase<MatchPhr
public void testIllegalValues() {
try {
new MatchPhraseQueryBuilder(null, "value");
fail("value must not be non-null");
fail("field must not be non-null");
} catch (IllegalArgumentException ex) {
// expected
assertEquals("[match_phrase] requires fieldName", ex.getMessage());
}
try {
new MatchPhraseQueryBuilder("fieldName", null);
fail("value must not be non-null");
} catch (IllegalArgumentException ex) {
// expected
assertEquals("[match_phrase] requires query value", ex.getMessage());
}
}
@ -119,4 +136,24 @@ public class MatchPhraseQueryBuilderTests extends AbstractQueryTestCase<MatchPhr
MatchPhraseQueryBuilder qb = (MatchPhraseQueryBuilder) parseQuery(json1);
checkGeneratedJson(expected, qb);
}
public void testParseFailsWithMultipleFields() throws IOException {
String json = "{\n" +
" \"match_phrase\" : {\n" +
" \"message1\" : {\n" +
" \"query\" : \"this is a test\"\n" +
" },\n" +
" \"message2\" : {\n" +
" \"query\" : \"this is a test\"\n" +
" }\n" +
" }\n" +
"}";
try {
parseQuery(json);
fail("parseQuery should have failed");
} catch(ParsingException e) {
assertEquals("[match_phrase] query doesn't support multiple fields, found [message1] and [message2]", e.getMessage());
}
}
}