Slightly more efficient null handling and fix null test logic

Author: mjmbischoff

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvContainsAll.java

@@ -174,13 +174,15 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
                 right()
             );
         }
+        if(supersetType == ElementType.NULL || subsetType == ElementType.NULL) {
+            return new MvContainsAllNullEvaluator(toEvaluator.apply(right()));
+        }
         return switch (supersetType) {
             case BOOLEAN -> new MvContainsAllBooleanEvaluator.Factory(source(), toEvaluator.apply(left()), toEvaluator.apply(right()));
             case BYTES_REF -> new MvContainsAllBytesRefEvaluator.Factory(source(), toEvaluator.apply(left()), toEvaluator.apply(right()));
             case DOUBLE -> new MvContainsAllDoubleEvaluator.Factory(source(), toEvaluator.apply(left()), toEvaluator.apply(right()));
             case INT -> new MvContainsAllIntEvaluator.Factory(source(), toEvaluator.apply(left()), toEvaluator.apply(right()));
             case LONG -> new MvContainsAllLongEvaluator.Factory(source(), toEvaluator.apply(left()), toEvaluator.apply(right()));
-            case NULL -> new MvContainsAllNullEvaluator(toEvaluator.apply(right()));
             default -> throw EsqlIllegalArgumentException.illegalDataType(dataType());
         };
     }
@@ -287,11 +289,12 @@ interface ValueExtractor<BlockType extends Block, Type> {
         Type extractValue(BlockType block, int position);
     }
 
-    private record MvContainsAllNullEvaluator(ExpressionEvaluator.Factory toEvaluator) implements ExpressionEvaluator.Factory {
+    private record MvContainsAllNullEvaluator(ExpressionEvaluator.Factory subsetFieldEvaluator) implements ExpressionEvaluator.Factory {
+
         @Override
         public ExpressionEvaluator get(DriverContext context) {
             return new ExpressionEvaluator() {
-                final ExpressionEvaluator subsetField = toEvaluator.get(context);
+                final ExpressionEvaluator subsetField = subsetFieldEvaluator.get(context);
 
                 @Override
                 public Block eval(Page page) {
@@ -305,7 +308,17 @@ public Block eval(Page page) {
                 public void close() {
                     Releasables.closeExpectNoException(subsetField);
                 }
+
+                @Override
+                public String toString() {
+                    return "MvContainsAllNullEvaluator[" + "subsetField=" + subsetFieldEvaluator + "]";
+                }
             };
         }
+
+        @Override
+        public String toString() {
+            return "MvContainsAllNullEvaluator[" + "subsetField=" + subsetFieldEvaluator + "]";
+        }
     }
 }

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvContainsAllTests.java

@@ -19,14 +19,20 @@
 import org.elasticsearch.xpack.esql.core.type.DataType;
 import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase;
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
+import org.hamcrest.Matcher;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.function.Supplier;
 
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.randomLiteral;
 import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.CARTESIAN;
 import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.GEO;
+import static org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier.TypedData.MULTI_ROW_NULL;
+import static org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier.TypedData.NULL;
+import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.equalTo;
 
 public class MvContainsAllTests extends AbstractScalarFunctionTestCase {
@@ -42,7 +48,14 @@ public static Iterable<Object[]> parameters() {
         longs(suppliers);
         doubles(suppliers);
         bytesRefs(suppliers);
-        return parameterSuppliersFromTypedDataWithDefaultChecksNoErrors(true, suppliers);
+
+        return parameterSuppliersFromTypedData(anyNullIsNull(
+            suppliers,
+            (nullPosition, nullValueDataType, original) -> false
+                && nullValueDataType == DataType.NULL
+                && original.getData().size() == 1 ? DataType.NULL : original.expectedType(),
+            (nullPosition, nullData, original) -> original
+        ));
     }
 
     @Override
@@ -270,4 +283,70 @@ private static void bytesRefs(List<TestCaseSupplier> suppliers) {
         }));
     }
 
+    // Adjusted from static method anyNullIsNull in {@code AbstractScalarFunctionTestCase#}
+    protected static List<TestCaseSupplier> anyNullIsNull(
+        List<TestCaseSupplier> testCaseSuppliers,
+        ExpectedType expectedType,
+        ExpectedEvaluatorToString evaluatorToString
+    ) {
+        List<TestCaseSupplier> suppliers = new ArrayList<>(testCaseSuppliers.size());
+        suppliers.addAll(testCaseSuppliers);
+
+        /*
+         * For each original test case, add as many copies as there were
+         * arguments, replacing one of the arguments with null and keeping
+         * the others.
+         *
+         * Also, if this was the first time we saw the signature we copy it
+         * *again*, replacing the argument with null, but annotating the
+         * argument’s type as `null` explicitly.
+         */
+        Set<List<DataType>> uniqueSignatures = new HashSet<>();
+        for (TestCaseSupplier original : testCaseSuppliers) {
+            boolean firstTimeSeenSignature = uniqueSignatures.add(original.types());
+            for (int typeIndex = 0; typeIndex < original.types().size(); typeIndex++) {
+                int nullPosition = typeIndex;
+
+                suppliers.add(new TestCaseSupplier(original.name() + " null in " + nullPosition, original.types(), () -> {
+                    TestCaseSupplier.TestCase originalTestCase = original.get();
+                    List<TestCaseSupplier.TypedData> data = new ArrayList<>(originalTestCase.getData());
+                    data.set(nullPosition, NULL);
+                    TestCaseSupplier.TypedData nulledData = originalTestCase.getData().get(nullPosition);
+                    return new TestCaseSupplier.TestCase(
+                        data,
+                        evaluatorToString.evaluatorToString(nullPosition, nulledData, originalTestCase.evaluatorToString()),
+                        expectedType.expectedType(nullPosition, nulledData.type(), originalTestCase),
+                        equalTo(nullPosition == 1)
+                    );
+                }));
+
+                if (firstTimeSeenSignature) {
+                    var typesWithNull = new ArrayList<>(original.types());
+                    typesWithNull.set(nullPosition, DataType.NULL);
+                    boolean newSignature = uniqueSignatures.add(typesWithNull);
+                    if (newSignature) {
+                        suppliers.add(new TestCaseSupplier(typesWithNull, () -> {
+                            TestCaseSupplier.TestCase originalTestCase = original.get();
+                            var typeDataWithNull = new ArrayList<>(originalTestCase.getData());
+                            typeDataWithNull.set(nullPosition, typeDataWithNull.get(nullPosition).isMultiRow() ? MULTI_ROW_NULL : NULL);
+                            return new TestCaseSupplier.TestCase(
+                                typeDataWithNull,
+                                "MvContainsAllNullEvaluator[subsetField=Attribute[channel=1]]",
+                                DataType.BOOLEAN,
+                                equalTo(nullPosition == 1)
+                            );
+                        }));
+                    }
+                }
+            }
+        }
+
+        return suppliers;
+    }
+
+    // We always return a boolean.
+    @Override
+    protected Matcher<Object> allNullsMatcher() {
+        return anyOf(equalTo(false),equalTo(true));
+    }
 }