Throw parsing error if regexp query contains multiple fields
Regexp Query, like many other queries, used to parse even when the query referred to multiple fields and the last one would win. We rather throw an exception now instead. Also added test for short prefix query variant.
This commit is contained in:
parent
69c2deedc7
commit
003a7b6eb3
|
@ -77,7 +77,7 @@ public class RegexpQueryBuilder extends AbstractQueryBuilder<RegexpQueryBuilder>
|
|||
throw new IllegalArgumentException("field name is null or empty");
|
||||
}
|
||||
if (value == null) {
|
||||
throw new IllegalArgumentException("value cannot be null.");
|
||||
throw new IllegalArgumentException("value cannot be null");
|
||||
}
|
||||
this.fieldName = fieldName;
|
||||
this.value = value;
|
||||
|
@ -180,10 +180,8 @@ public class RegexpQueryBuilder extends AbstractQueryBuilder<RegexpQueryBuilder>
|
|||
|
||||
public static Optional<RegexpQueryBuilder> fromXContent(QueryParseContext parseContext) throws IOException {
|
||||
XContentParser parser = parseContext.parser();
|
||||
|
||||
String fieldName = parser.currentName();
|
||||
String fieldName = null;
|
||||
String rewrite = null;
|
||||
|
||||
String value = null;
|
||||
float boost = AbstractQueryBuilder.DEFAULT_BOOST;
|
||||
int flagsValue = RegexpQueryBuilder.DEFAULT_FLAGS_VALUE;
|
||||
|
@ -197,6 +195,10 @@ public class RegexpQueryBuilder extends AbstractQueryBuilder<RegexpQueryBuilder>
|
|||
} else if (parseContext.isDeprecatedSetting(currentFieldName)) {
|
||||
// skip
|
||||
} else if (token == XContentParser.Token.START_OBJECT) {
|
||||
if (fieldName != null) {
|
||||
throw new ParsingException(parser.getTokenLocation(), "[regexp] 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) {
|
||||
|
@ -233,9 +235,6 @@ public class RegexpQueryBuilder extends AbstractQueryBuilder<RegexpQueryBuilder>
|
|||
}
|
||||
}
|
||||
|
||||
if (value == null) {
|
||||
throw new ParsingException(parser.getTokenLocation(), "No value specified for regexp query");
|
||||
}
|
||||
return Optional.of(new RegexpQueryBuilder(fieldName, value)
|
||||
.flags(flagsValue)
|
||||
.maxDeterminizedStates(maxDeterminizedStates)
|
||||
|
|
|
@ -21,11 +21,14 @@ package org.elasticsearch.index.query;
|
|||
|
||||
import org.apache.lucene.search.Query;
|
||||
import org.apache.lucene.search.RegexpQuery;
|
||||
import org.elasticsearch.common.ParsingException;
|
||||
import org.elasticsearch.test.AbstractQueryTestCase;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.hamcrest.Matchers.instanceOf;
|
||||
|
@ -34,11 +37,7 @@ public class RegexpQueryBuilderTests extends AbstractQueryTestCase<RegexpQueryBu
|
|||
|
||||
@Override
|
||||
protected RegexpQueryBuilder doCreateTestQueryBuilder() {
|
||||
// mapped or unmapped fields
|
||||
String fieldName = randomBoolean() ? STRING_FIELD_NAME : randomAsciiOfLengthBetween(1, 10);
|
||||
String value = randomAsciiOfLengthBetween(1, 10);
|
||||
RegexpQueryBuilder query = new RegexpQueryBuilder(fieldName, value);
|
||||
|
||||
RegexpQueryBuilder query = randomRegexpQuery();
|
||||
if (randomBoolean()) {
|
||||
List<RegexpFlag> flags = new ArrayList<>();
|
||||
int iter = randomInt(5);
|
||||
|
@ -56,6 +55,26 @@ public class RegexpQueryBuilderTests extends AbstractQueryTestCase<RegexpQueryBu
|
|||
return query;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Map<String, RegexpQueryBuilder> getAlternateVersions() {
|
||||
Map<String, RegexpQueryBuilder> alternateVersions = new HashMap<>();
|
||||
RegexpQueryBuilder regexpQuery = randomRegexpQuery();
|
||||
String contentString = "{\n" +
|
||||
" \"regexp\" : {\n" +
|
||||
" \"" + regexpQuery.fieldName() + "\" : \"" + regexpQuery.value() + "\"\n" +
|
||||
" }\n" +
|
||||
"}";
|
||||
alternateVersions.put(contentString, regexpQuery);
|
||||
return alternateVersions;
|
||||
}
|
||||
|
||||
private static RegexpQueryBuilder randomRegexpQuery() {
|
||||
// mapped or unmapped fields
|
||||
String fieldName = randomBoolean() ? STRING_FIELD_NAME : randomAsciiOfLengthBetween(1, 10);
|
||||
String value = randomAsciiOfLengthBetween(1, 10);
|
||||
return new RegexpQueryBuilder(fieldName, value);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doAssertLuceneQuery(RegexpQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
assertThat(query, instanceOf(RegexpQuery.class));
|
||||
|
@ -72,14 +91,14 @@ public class RegexpQueryBuilderTests extends AbstractQueryTestCase<RegexpQueryBu
|
|||
}
|
||||
fail("cannot be null or empty");
|
||||
} catch (IllegalArgumentException e) {
|
||||
// expected
|
||||
assertEquals("field name is null or empty", e.getMessage());
|
||||
}
|
||||
|
||||
try {
|
||||
new RegexpQueryBuilder("field", null);
|
||||
fail("cannot be null or empty");
|
||||
} catch (IllegalArgumentException e) {
|
||||
// expected
|
||||
assertEquals("value cannot be null", e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -112,4 +131,25 @@ public class RegexpQueryBuilderTests extends AbstractQueryTestCase<RegexpQueryBu
|
|||
assertEquals("Can only use regexp queries on keyword and text fields - not on [mapped_int] which is of type [integer]",
|
||||
e.getMessage());
|
||||
}
|
||||
|
||||
public void testParseFailsWithMultipleFields() throws IOException {
|
||||
String json =
|
||||
"{\n" +
|
||||
" \"regexp\": {\n" +
|
||||
" \"user1\": {\n" +
|
||||
" \"value\": \"k.*y\"\n" +
|
||||
" },\n" +
|
||||
" \"user2\": {\n" +
|
||||
" \"value\": \"k.*y\"\n" +
|
||||
" }\n" +
|
||||
" }\n" +
|
||||
"}";
|
||||
|
||||
try {
|
||||
parseQuery(json);
|
||||
fail("parseQuery should have failed");
|
||||
} catch(ParsingException e) {
|
||||
assertEquals("[regexp] query doesn't support multiple fields, found [user1] and [user2]", e.getMessage());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue