Skip to content

Commit

Permalink
Fix bool query behaviour on null value (#56817)
Browse files Browse the repository at this point in the history
Until 7.7 we used to ignore `null` values for `bool`queries `minimum_should_match`,
parameters and also for the `must`,  `must_not`, `should` and `filter` clauses.
An internal refactoring has changed this so now we get a parsing error. While `null` 
should not a common value here, we should restore the old behaviour for bwc for now.

Closes #56812
  • Loading branch information
Christoph Büscher committed May 26, 2020
1 parent 03a0dc7 commit c6cd0e6
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,24 @@ public <T> void declareObjectArray(BiConsumer<Value, List<T>> consumer, ContextP
declareFieldArray(consumer, (p, c) -> objectParser.parse(p, c), field, ValueType.OBJECT_ARRAY);
}

/**
* like {@link #declareObjectArray(BiConsumer, ContextParser, ParseField)}, but can also handle single null values,
* in which case the consumer isn't called
*/
public <
T> void declareObjectArrayOrNull(
BiConsumer<Value, List<T>> consumer,
ContextParser<Context, T> objectParser,
ParseField field
) {
declareField(
(value, list) -> { if (list != null) consumer.accept(value, list); },
(p, c) -> p.currentToken() == XContentParser.Token.VALUE_NULL ? null : parseArray(p, () -> objectParser.parse(p, c)),
field,
ValueType.OBJECT_ARRAY_OR_NULL
);
}

public void declareStringArray(BiConsumer<Value, List<String>> consumer, ParseField field) {
declareFieldArray(consumer, (p, c) -> p.text(), field, ValueType.STRING_ARRAY);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ public enum ValueType {
OBJECT(START_OBJECT),
OBJECT_OR_NULL(START_OBJECT, VALUE_NULL),
OBJECT_ARRAY(START_OBJECT, START_ARRAY),
OBJECT_ARRAY_OR_NULL(START_OBJECT, START_ARRAY, VALUE_NULL),
OBJECT_OR_BOOLEAN(START_OBJECT, VALUE_BOOLEAN),
OBJECT_OR_STRING(START_OBJECT, VALUE_STRING),
OBJECT_OR_LONG(START_OBJECT, VALUE_NUMBER),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,16 +278,16 @@ private static void doXArrayContent(ParseField field, List<QueryBuilder> clauses

private static final ObjectParser<BoolQueryBuilder, Void> PARSER = new ObjectParser<>("bool", BoolQueryBuilder::new);
static {
PARSER.declareObjectArray((builder, clauses) -> clauses.forEach(builder::must), (p, c) -> parseInnerQueryBuilder(p),
PARSER.declareObjectArrayOrNull((builder, clauses) -> clauses.forEach(builder::must), (p, c) -> parseInnerQueryBuilder(p),
MUST);
PARSER.declareObjectArray((builder, clauses) -> clauses.forEach(builder::should), (p, c) -> parseInnerQueryBuilder(p),
PARSER.declareObjectArrayOrNull((builder, clauses) -> clauses.forEach(builder::should), (p, c) -> parseInnerQueryBuilder(p),
SHOULD);
PARSER.declareObjectArray((builder, clauses) -> clauses.forEach(builder::mustNot), (p, c) -> parseInnerQueryBuilder(p),
PARSER.declareObjectArrayOrNull((builder, clauses) -> clauses.forEach(builder::mustNot), (p, c) -> parseInnerQueryBuilder(p),
MUST_NOT);
PARSER.declareObjectArray((builder, clauses) -> clauses.forEach(builder::filter), (p, c) -> parseInnerQueryBuilder(p),
PARSER.declareObjectArrayOrNull((builder, clauses) -> clauses.forEach(builder::filter), (p, c) -> parseInnerQueryBuilder(p),
FILTER);
PARSER.declareBoolean(BoolQueryBuilder::adjustPureNegative, ADJUST_PURE_NEGATIVE);
PARSER.declareField(BoolQueryBuilder::minimumShouldMatch, (p, c) -> p.text(),
PARSER.declareField(BoolQueryBuilder::minimumShouldMatch, (p, c) -> p.textOrNull(),
MINIMUM_SHOULD_MATCH, ObjectParser.ValueType.VALUE);
PARSER.declareString(BoolQueryBuilder::queryName, NAME_FIELD);
PARSER.declareFloat(BoolQueryBuilder::boost, BOOST_FIELD);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,36 @@ public void testMinimumShouldMatchNumber() throws IOException {
assertEquals("1", builder.minimumShouldMatch());
}

public void testMinimumShouldMatchNull() throws IOException {
String query = "{\"bool\" : {\"must\" : { \"term\" : { \"field\" : \"value\" } }, \"minimum_should_match\" : null } }";
BoolQueryBuilder builder = (BoolQueryBuilder) parseQuery(query);
assertEquals(null, builder.minimumShouldMatch());
}

public void testMustNull() throws IOException {
String query = "{\"bool\" : {\"must\" : null } }";
BoolQueryBuilder builder = (BoolQueryBuilder) parseQuery(query);
assertTrue(builder.must().isEmpty());
}

public void testMustNotNull() throws IOException {
String query = "{\"bool\" : {\"must_not\" : null } }";
BoolQueryBuilder builder = (BoolQueryBuilder) parseQuery(query);
assertTrue(builder.mustNot().isEmpty());
}

public void testShouldNull() throws IOException {
String query = "{\"bool\" : {\"should\" : null } }";
BoolQueryBuilder builder = (BoolQueryBuilder) parseQuery(query);
assertTrue(builder.should().isEmpty());
}

public void testFilterNull() throws IOException {
String query = "{\"bool\" : {\"filter\" : null } }";
BoolQueryBuilder builder = (BoolQueryBuilder) parseQuery(query);
assertTrue(builder.filter().isEmpty());
}

/**
* test that unknown query names in the clauses throw an error
*/
Expand Down

0 comments on commit c6cd0e6

Please sign in to comment.