Merge pull request #4770 from gchq/gh-4767_null_equality

#4767 Improve null equality treatment in expressions
gchq · Feb 7, 2025 · 89921fe · 89921fe
2 parents 5ff45c6 + dccf05d
commit 89921fe
Show file tree

Hide file tree

Showing 19 changed files with 217 additions and 78 deletions.
diff --git a/...ery-language/src/main/java/stroom/query/language/functions/AbstractAggregateFunction.java b/...ery-language/src/main/java/stroom/query/language/functions/AbstractAggregateFunction.java
@@ -109,9 +109,9 @@ public Val eval(final StoredValues storedValues, final Supplier<ChildData> child
             Val value = ValNull.INSTANCE;
             for (final Generator gen : childGenerators) {
                 final Val val = gen.eval(storedValues, childDataSupplier);
-                if (!val.type().isValue()) {
-                    return val;
-                }
+//                if (!val.type().isValue()) {
+//                    return val;
+//                }
                 value = calculator.calc(value, val);
             }
             return value;

diff --git a/...uery-language/src/main/java/stroom/query/language/functions/AbstractEqualityFunction.java b/...uery-language/src/main/java/stroom/query/language/functions/AbstractEqualityFunction.java
@@ -24,7 +24,6 @@
 abstract class AbstractEqualityFunction extends AbstractManyChildFunction {
 
     private static final ValErr CHILD_ERROR = ValErr.create("Error evaluating child generator");
-    private static final ValErr MISSING_VALUE = ValErr.create("Both values must have a value to test equality");
 
     private final boolean usingOperator;
 
@@ -91,8 +90,8 @@ public Val eval(final StoredValues storedValues, final Supplier<ChildData> child
                     return CHILD_ERROR;
                 }
 
-                if (!val.type().isValue()) {
-                    return ValErr.wrap(val, MISSING_VALUE);
+                if (val.type().isError()) {
+                    return val;
                 }
 
                 values[i] = val;

diff --git a/stroom-query/stroom-query-language/src/main/java/stroom/query/language/functions/Add.java b/stroom-query/stroom-query-language/src/main/java/stroom/query/language/functions/Add.java
@@ -28,7 +28,7 @@
         commonReturnType = ValDouble.class,
         commonReturnDescription = "The sum of all values",
         commonDescription = "Adds the value of all arguments together. Minimum of two arguments. Can be expressed as " +
-                "'${field1}+${field2}'.",
+                            "'${field1}+${field2}'.",
         signatures = @FunctionSignature(
                 args = @FunctionArg(
                         name = "arg",
@@ -64,9 +64,13 @@ static class Calc extends Calculator {
         Val calc(final Val current, final Val value) {
             try {
                 if (Type.DURATION.equals(value.type())) {
-                    if (!current.type().isValue() || value.type().isError()) {
+                    if (value.type().isError() ||
+                        current.type().isNull()) {
                         return value;
+                    } else if (current.type().isError()) {
+                        return current;
                     }
+
                     final long milliseconds = value.toLong();
                     final long diff = current.toLong() + milliseconds;
                     if (Type.DATE.equals(current.type())) {

diff --git a/...query/stroom-query-language/src/main/java/stroom/query/language/functions/Calculator.java b/...query/stroom-query-language/src/main/java/stroom/query/language/functions/Calculator.java
@@ -21,8 +21,11 @@ abstract class Calculator {
 
     Val calc(final Val current, final Val value) {
         try {
-            if (value.type().isError()) {
+            if (value.type().isError() ||
+                current.type().isNull()) {
                 return value;
+            } else if (current.type().isError()) {
+                return current;
             }
 
             final Double cur = current.toDouble();

diff --git a/stroom-query/stroom-query-language/src/main/java/stroom/query/language/functions/Equals.java b/stroom-query/stroom-query-language/src/main/java/stroom/query/language/functions/Equals.java
@@ -16,6 +16,8 @@
 
 package stroom.query.language.functions;
 
+import java.util.Objects;
+
 @SuppressWarnings("unused") //Used by FunctionFactory
 @FunctionDef(
         name = Equals.NAME,
@@ -59,6 +61,9 @@ private static class EqualsEvaluator extends Evaluator {
 
         @Override
         protected Val evaluate(final Val a, final Val b) {
+            if (Objects.equals(a, b)) {
+                return ValBoolean.TRUE;
+            }
 
             final int compareResult = ValComparators.GENERIC_CASE_SENSITIVE_COMPARATOR.compare(a, b);
             return compareResult == 0

diff --git a/...uery/stroom-query-language/src/main/java/stroom/query/language/functions/GreaterThan.java b/...uery/stroom-query-language/src/main/java/stroom/query/language/functions/GreaterThan.java
@@ -59,6 +59,9 @@ private static class GreaterThanEvaluator extends Evaluator {
 
         @Override
         protected Val evaluate(final Val a, final Val b) {
+            if (a.type().isNull() || b.type().isNull()) {
+                return ValBoolean.FALSE;
+            }
 
             final int compareResult = ValComparators.GENERIC_CASE_SENSITIVE_COMPARATOR.compare(a, b);
             return compareResult > 0

diff --git a/...om-query-language/src/main/java/stroom/query/language/functions/GreaterThanOrEqualTo.java b/...om-query-language/src/main/java/stroom/query/language/functions/GreaterThanOrEqualTo.java
@@ -16,6 +16,8 @@
 
 package stroom.query.language.functions;
 
+import java.util.Objects;
+
 @SuppressWarnings("unused") //Used by FunctionFactory
 @FunctionDef(
         name = GreaterThanOrEqualTo.NAME,
@@ -60,6 +62,13 @@ private static class GreaterThanEvaluator extends Evaluator {
 
         @Override
         protected Val evaluate(final Val a, final Val b) {
+            if (a.type().isNull() || b.type().isNull()) {
+                if (Objects.equals(a, b)) {
+                    return ValBoolean.TRUE;
+                }
+                return ValBoolean.FALSE;
+            }
+
             final int compareResult = ValComparators.GENERIC_CASE_SENSITIVE_COMPARATOR.compare(a, b);
             return compareResult >= 0
                     ? ValBoolean.TRUE

diff --git a/...m-query/stroom-query-language/src/main/java/stroom/query/language/functions/LessThan.java b/...m-query/stroom-query-language/src/main/java/stroom/query/language/functions/LessThan.java
@@ -59,6 +59,10 @@ private static class LessThanEvaluator extends Evaluator {
 
         @Override
         protected Val evaluate(final Val a, final Val b) {
+            if (a.type().isNull() || b.type().isNull()) {
+                return ValBoolean.FALSE;
+            }
+
             final int compareResult = ValComparators.GENERIC_CASE_SENSITIVE_COMPARATOR.compare(a, b);
             return compareResult < 0
                     ? ValBoolean.TRUE

diff --git a/...troom-query-language/src/main/java/stroom/query/language/functions/LessThanOrEqualTo.java b/...troom-query-language/src/main/java/stroom/query/language/functions/LessThanOrEqualTo.java
@@ -16,6 +16,8 @@
 
 package stroom.query.language.functions;
 
+import java.util.Objects;
+
 @SuppressWarnings("unused") //Used by FunctionFactory
 @FunctionDef(
         name = LessThanOrEqualTo.NAME,
@@ -59,6 +61,12 @@ private static class LessThanOrEqualToEvaluator extends Evaluator {
 
         @Override
         protected Val evaluate(final Val a, final Val b) {
+            if (a.type().isNull() || b.type().isNull()) {
+                if (Objects.equals(a, b)) {
+                    return ValBoolean.TRUE;
+                }
+                return ValBoolean.FALSE;
+            }
 
             final int compareResult = ValComparators.GENERIC_CASE_SENSITIVE_COMPARATOR.compare(a, b);
             return compareResult <= 0

diff --git a/...-language/src/test/java/stroom/query/language/functions/AbstractEqualityFunctionTest.java b/...-language/src/test/java/stroom/query/language/functions/AbstractEqualityFunctionTest.java
@@ -28,17 +28,17 @@ Stream<TestCase> getTestCases() {
                 .flatMap(values -> {
                     final Object param1 = values.param1;
                     final Object param2 = values.param2;
-                    if (!addInverseTest()) {
+                    if (!addInverseTest() || param1 == null || param2 == null) {
                         return Stream.of(values);
                     } else if (ValComparators.haveType(param1, param2, String.class)
-                            && String.CASE_INSENSITIVE_ORDER.compare(
+                               && String.CASE_INSENSITIVE_ORDER.compare(
                             param1.toString(),
                             param1.toString()) == 0) {
                         return Stream.of(values);
                     } else if (Objects.equals(param1, param2)) {
                         return Stream.of(values);
                     } else if ((param1 instanceof Number
-                            && DoubleMath.fuzzyEquals(
+                                && DoubleMath.fuzzyEquals(
                             ((Number) param1).doubleValue(),
                             ((Number) param2).doubleValue(),
                             Val.FLOATING_POINT_EQUALITY_TOLERANCE))) {
@@ -61,17 +61,24 @@ protected TestCase buildCase(final Object param1,
                                  final boolean expectedOutcome) {
         return TestCase.convert(
                 param1
-                        + " (" + param1.getClass().getSimpleName() + ") "
-                        + getOperator() + " "
-                        + param2
-                        + " (" + param2.getClass().getSimpleName() + ")"
-                        + " => "
-                        + expectedOutcome,
+                + " (" + getParamType(param1) + ") "
+                + getOperator() + " "
+                + param2
+                + " (" + getParamType(param2) + ")"
+                + " => "
+                + expectedOutcome,
                 expectedOutcome,
                 param1,
                 param2);
     }
 
+    private String getParamType(final Object param) {
+        if (param == null) {
+            return "null";
+        }
+        return param.getClass().getSimpleName();
+    }
+
 
     // --------------------------------------------------------------------------------
 

diff --git a/...query/stroom-query-language/src/test/java/stroom/query/language/functions/TestEquals.java b/...query/stroom-query-language/src/test/java/stroom/query/language/functions/TestEquals.java
@@ -33,6 +33,7 @@ Stream<Values> getTestCaseValues() {
                 Values.of(2, 2.0D, true),
                 Values.of(2, 2F, true),
                 Values.of(2, 2.0F, true),
+                Values.of(2, null, false),
 
                 Values.of(2L, 1L, false),
                 Values.of(2L, 1, false),
@@ -44,6 +45,7 @@ Stream<Values> getTestCaseValues() {
                 Values.of(2L, 2.0D, true),
                 Values.of(2L, 2F, true),
                 Values.of(2L, 2.0F, true),
+                Values.of(2L, null, false),
 
                 Values.of(1.2D, 1.1D, false),
                 Values.of(1.2D, 1, false),
@@ -54,6 +56,7 @@ Stream<Values> getTestCaseValues() {
                 Values.of(1D, 1, true),
                 Values.of(1D, 1L, true),
                 Values.of(1.1D, 1.1F, true),
+                Values.of(1.1D, null, false),
 
                 Values.of(1.2F, 1.1F, false),
                 Values.of(1.2F, 1, false),
@@ -63,16 +66,20 @@ Stream<Values> getTestCaseValues() {
                 Values.of(1F, 1, true),
                 Values.of(1F, 1L, true),
                 Values.of(1.1F, 1.1D, true),
+                Values.of(1.1F, null, false),
 
                 Values.of(true, false, false),
                 Values.of(true, true, true),
+                Values.of(true, null, false),
 
                 Values.of("dog", "cat", false),
                 Values.of("CAT", "cat", false),
                 Values.of("cat", "cat", true),
+                Values.of("cat", null, false),
 
                 Values.of("1", "1", true),
                 Values.of("1", "2", false),
+                Values.of("1", null, false),
 
                 Values.of(true, "true", true),
                 Values.of(true, 1, true),
@@ -81,17 +88,21 @@ Stream<Values> getTestCaseValues() {
                 Values.of(true, 1.0F, true),
                 Values.of(true, 1D, true),
                 Values.of(true, 1.0D, true),
+                Values.of(true, null, false),
 
                 Values.of(false, "false", true),
                 Values.of(false, 0, true),
                 Values.of(false, 0L, true),
                 Values.of(false, 0F, true),
                 Values.of(false, 0.0F, true),
                 Values.of(false, 0.0D, true),
+                Values.of(false, null, false),
 
                 Values.of(Duration.ofSeconds(2), Duration.ofSeconds(1), false),
                 Values.of(Duration.ofSeconds(1), Duration.ofSeconds(1), true),
-                Values.of(Duration.ofSeconds(1), 1_000, true)
+                Values.of(Duration.ofSeconds(1), 1_000, true),
+
+                Values.of(null, null, true)
         );
     }
 }