mirror of
https://github.com/honeymoose/OpenSearch.git
synced 2025-02-20 03:45:02 +00:00
Throw parsing error if match_phrase_prefix query contains multiple fields
Match phrase prefix 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:
parent
ad8f5e7e4b
commit
f7b3dce4bc
@ -192,23 +192,26 @@ public class MatchPhrasePrefixQueryBuilder extends AbstractQueryBuilder<MatchPhr
|
|||||||
|
|
||||||
public static Optional<MatchPhrasePrefixQueryBuilder> fromXContent(QueryParseContext parseContext) throws IOException {
|
public static Optional<MatchPhrasePrefixQueryBuilder> fromXContent(QueryParseContext parseContext) throws IOException {
|
||||||
XContentParser parser = parseContext.parser();
|
XContentParser parser = parseContext.parser();
|
||||||
|
String fieldName = null;
|
||||||
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();
|
|
||||||
|
|
||||||
Object value = null;
|
Object value = null;
|
||||||
float boost = AbstractQueryBuilder.DEFAULT_BOOST;
|
float boost = AbstractQueryBuilder.DEFAULT_BOOST;
|
||||||
String analyzer = null;
|
String analyzer = null;
|
||||||
int slop = MatchQuery.DEFAULT_PHRASE_SLOP;
|
int slop = MatchQuery.DEFAULT_PHRASE_SLOP;
|
||||||
int maxExpansion = FuzzyQuery.defaultMaxExpansions;
|
int maxExpansion = FuzzyQuery.defaultMaxExpansions;
|
||||||
String queryName = null;
|
String queryName = null;
|
||||||
|
XContentParser.Token token;
|
||||||
token = parser.nextToken();
|
|
||||||
if (token == XContentParser.Token.START_OBJECT) {
|
|
||||||
String currentFieldName = null;
|
String currentFieldName = null;
|
||||||
|
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_prefix] query doesn't support multiple " +
|
||||||
|
"fields, found [" + fieldName + "] and [" + currentFieldName + "]");
|
||||||
|
}
|
||||||
|
fieldName = currentFieldName;
|
||||||
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
|
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
|
||||||
if (token == XContentParser.Token.FIELD_NAME) {
|
if (token == XContentParser.Token.FIELD_NAME) {
|
||||||
currentFieldName = parser.currentName();
|
currentFieldName = parser.currentName();
|
||||||
@ -234,22 +237,12 @@ public class MatchPhrasePrefixQueryBuilder extends AbstractQueryBuilder<MatchPhr
|
|||||||
"[" + NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]");
|
"[" + NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
parser.nextToken();
|
|
||||||
} else {
|
} else {
|
||||||
|
fieldName = parser.currentName();
|
||||||
value = parser.objectText();
|
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");
|
|
||||||
}
|
|
||||||
|
|
||||||
MatchPhrasePrefixQueryBuilder matchQuery = new MatchPhrasePrefixQueryBuilder(fieldName, value);
|
MatchPhrasePrefixQueryBuilder matchQuery = new MatchPhrasePrefixQueryBuilder(fieldName, value);
|
||||||
matchQuery.analyzer(analyzer);
|
matchQuery.analyzer(analyzer);
|
||||||
matchQuery.slop(slop);
|
matchQuery.slop(slop);
|
||||||
|
@ -23,11 +23,15 @@ import org.apache.lucene.search.BooleanQuery;
|
|||||||
import org.apache.lucene.search.PointRangeQuery;
|
import org.apache.lucene.search.PointRangeQuery;
|
||||||
import org.apache.lucene.search.Query;
|
import org.apache.lucene.search.Query;
|
||||||
import org.apache.lucene.search.TermQuery;
|
import org.apache.lucene.search.TermQuery;
|
||||||
|
import org.elasticsearch.common.ParsingException;
|
||||||
import org.elasticsearch.common.lucene.search.MatchNoDocsQuery;
|
import org.elasticsearch.common.lucene.search.MatchNoDocsQuery;
|
||||||
import org.elasticsearch.common.lucene.search.MultiPhrasePrefixQuery;
|
import org.elasticsearch.common.lucene.search.MultiPhrasePrefixQuery;
|
||||||
import org.elasticsearch.test.AbstractQueryTestCase;
|
import org.elasticsearch.test.AbstractQueryTestCase;
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
|
import java.util.HashMap;
|
||||||
|
import java.util.Map;
|
||||||
|
|
||||||
import static org.hamcrest.CoreMatchers.either;
|
import static org.hamcrest.CoreMatchers.either;
|
||||||
import static org.hamcrest.CoreMatchers.instanceOf;
|
import static org.hamcrest.CoreMatchers.instanceOf;
|
||||||
import static org.hamcrest.Matchers.containsString;
|
import static org.hamcrest.Matchers.containsString;
|
||||||
@ -69,6 +73,20 @@ public class MatchPhrasePrefixQueryBuilderTests extends AbstractQueryTestCase<Ma
|
|||||||
return matchQuery;
|
return matchQuery;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
protected Map<String, MatchPhrasePrefixQueryBuilder> getAlternateVersions() {
|
||||||
|
Map<String, MatchPhrasePrefixQueryBuilder> alternateVersions = new HashMap<>();
|
||||||
|
MatchPhrasePrefixQueryBuilder matchPhrasePrefixQuery = new MatchPhrasePrefixQueryBuilder(randomAsciiOfLengthBetween(1, 10),
|
||||||
|
randomAsciiOfLengthBetween(1, 10));
|
||||||
|
String contentString = "{\n" +
|
||||||
|
" \"match_phrase_prefix\" : {\n" +
|
||||||
|
" \"" + matchPhrasePrefixQuery.fieldName() + "\" : \"" + matchPhrasePrefixQuery.value() + "\"\n" +
|
||||||
|
" }\n" +
|
||||||
|
"}";
|
||||||
|
alternateVersions.put(contentString, matchPhrasePrefixQuery);
|
||||||
|
return alternateVersions;
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected void doAssertLuceneQuery(MatchPhrasePrefixQueryBuilder queryBuilder, Query query, QueryShardContext context)
|
protected void doAssertLuceneQuery(MatchPhrasePrefixQueryBuilder queryBuilder, Query query, QueryShardContext context)
|
||||||
throws IOException {
|
throws IOException {
|
||||||
@ -81,16 +99,16 @@ public class MatchPhrasePrefixQueryBuilderTests extends AbstractQueryTestCase<Ma
|
|||||||
public void testIllegalValues() {
|
public void testIllegalValues() {
|
||||||
try {
|
try {
|
||||||
new MatchPhrasePrefixQueryBuilder(null, "value");
|
new MatchPhrasePrefixQueryBuilder(null, "value");
|
||||||
fail("value must not be non-null");
|
fail("field must not be non-null");
|
||||||
} catch (IllegalArgumentException ex) {
|
} catch (IllegalArgumentException ex) {
|
||||||
// expected
|
assertEquals("[match_phrase_prefix] requires fieldName", ex.getMessage());
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
new MatchPhrasePrefixQueryBuilder("fieldName", null);
|
new MatchPhrasePrefixQueryBuilder("fieldName", null);
|
||||||
fail("value must not be non-null");
|
fail("value must not be non-null");
|
||||||
} catch (IllegalArgumentException ex) {
|
} catch (IllegalArgumentException ex) {
|
||||||
// expected
|
assertEquals("[match_phrase_prefix] requires query value", ex.getMessage());
|
||||||
}
|
}
|
||||||
|
|
||||||
MatchPhrasePrefixQueryBuilder matchQuery = new MatchPhrasePrefixQueryBuilder("fieldName", "text");
|
MatchPhrasePrefixQueryBuilder matchQuery = new MatchPhrasePrefixQueryBuilder("fieldName", "text");
|
||||||
@ -155,4 +173,25 @@ public class MatchPhrasePrefixQueryBuilderTests extends AbstractQueryTestCase<Ma
|
|||||||
qb = (MatchPhrasePrefixQueryBuilder) parseQuery(json3);
|
qb = (MatchPhrasePrefixQueryBuilder) parseQuery(json3);
|
||||||
checkGeneratedJson(expected, qb);
|
checkGeneratedJson(expected, qb);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
public void testParseFailsWithMultipleFields() throws IOException {
|
||||||
|
String json = "{\n" +
|
||||||
|
" \"match_phrase_prefix\" : {\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_prefix] query doesn't support multiple fields, found [message1] and [message2]", e.getMessage());
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user