Browse Source

Fix `bool` query behaviour on null value (#56817)

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
Christoph Büscher 5 years ago
parent
commit
1211699473

+ 18 - 0
libs/x-content/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java

@@ -240,6 +240,24 @@ public abstract class AbstractObjectParser<Value, Context> {
         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);
     }

+ 1 - 0
libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java

@@ -641,6 +641,7 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
         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),

+ 5 - 5
server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java

@@ -273,16 +273,16 @@ public class BoolQueryBuilder extends AbstractQueryBuilder<BoolQueryBuilder> {
 
     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);

+ 30 - 0
server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java

@@ -284,6 +284,36 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase<BoolQueryBuilde
         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
      */