From 8437ace02c748289f44cddc82be2ad09f0886656 Mon Sep 17 00:00:00 2001 From: Sean Huh Date: Fri, 30 Oct 2026 15:51:01 -0700 Subject: [PATCH] Align scalar numeric comparisons and relational operators with IEEE 754 PiperOrigin-RevId: 1010850635 --- .../java/dev/cel/common/internal/BUILD.bazel | 1 - .../common/internal/ComparisonFunctions.java | 79 ++++--------- .../internal/ComparisonFunctionsTest.java | 45 ++++++++ .../test/java/dev/cel/conformance/BUILD.bazel | 42 +------ .../extensions/CelListsExtensionsTest.java | 5 + .../cel/extensions/CelMathExtensionsTest.java | 4 + .../standard/GreaterEqualsOperator.java | 9 +- .../cel/runtime/standard/GreaterOperator.java | 9 +- .../runtime/standard/LessEqualsOperator.java | 9 +- .../cel/runtime/standard/LessOperator.java | 9 +- .../runtime/planner/ProgramPlannerTest.java | 109 +++++++++++++++++- 11 files changed, 207 insertions(+), 114 deletions(-) diff --git a/common/src/main/java/dev/cel/common/internal/BUILD.bazel b/common/src/main/java/dev/cel/common/internal/BUILD.bazel index 58b15b103..409ddfb8f 100644 --- a/common/src/main/java/dev/cel/common/internal/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/internal/BUILD.bazel @@ -88,7 +88,6 @@ java_library( tags = [ ], deps = [ - "//:auto_value", "//common/annotations", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", diff --git a/common/src/main/java/dev/cel/common/internal/ComparisonFunctions.java b/common/src/main/java/dev/cel/common/internal/ComparisonFunctions.java index 4d498d526..0fe1720ca 100644 --- a/common/src/main/java/dev/cel/common/internal/ComparisonFunctions.java +++ b/common/src/main/java/dev/cel/common/internal/ComparisonFunctions.java @@ -30,18 +30,9 @@ public final class ComparisonFunctions { private static final UnsignedLong UINT_TO_INT_MAX = UnsignedLong.valueOf(Long.MAX_VALUE); - private static final double DOUBLE_TO_INT_MAX = (double) Long.MAX_VALUE; - private static final double DOUBLE_TO_INT_MIN = (double) Long.MIN_VALUE; - private static final double DOUBLE_TO_UINT_MAX = UnsignedLong.MAX_VALUE.doubleValue(); public static int compareDoubleInt(double d, long l) { - if (d < DOUBLE_TO_INT_MIN) { - return -1; - } - if (d > DOUBLE_TO_INT_MAX) { - return 1; - } - return Double.compare(d, (double) l); + return compareDouble(d, (double) l); } public static int compareIntDouble(long l, double d) { @@ -49,13 +40,7 @@ public static int compareIntDouble(long l, double d) { } public static int compareDoubleUint(double d, UnsignedLong ul) { - if (d < 0.0) { - return -1; - } - if (d > DOUBLE_TO_UINT_MAX) { - return 1; - } - return Double.compare(d, ul.doubleValue()); + return compareDouble(d, ul.doubleValue()); } public static int compareUintDouble(UnsignedLong ul, double d) { @@ -73,54 +58,33 @@ public static int compareUintInt(UnsignedLong ul, long l) { return -compareIntUint(l, ul); } + /** Returns true if {@code value} is one of CEL's canonical runtime numeric types. */ + public static boolean isNumeric(Object value) { + return value instanceof Long || value instanceof Double || value instanceof UnsignedLong; + } + /** * Compare two numeric values of any type (double, int, uint) for equality. * - *

Floating point values are follow IEEE 754 standard for NaN comparisons. + *

Floating point values follow IEEE 754 standard for NaN and negative zero comparisons. */ public static boolean numericEquals(Number x, Number y) { - if (x instanceof Double) { - if (y instanceof Double) { - return !(Double.isNaN((Double) x) || Double.isNaN((Double) y)) - && x.doubleValue() == y.doubleValue(); - } - if (y instanceof Long) { - return compareDoubleInt((Double) x, (Long) y) == 0; - } - if (y instanceof UnsignedLong) { - return compareDoubleUint((Double) x, (UnsignedLong) y) == 0; - } - } - if (x instanceof Long) { - if (y instanceof Long) { - return x.equals(y); - } - if (y instanceof Double) { - return compareIntDouble((Long) x, (Double) y) == 0; - } - if (y instanceof UnsignedLong) { - return compareIntUint((Long) x, (UnsignedLong) y) == 0; - } - } - if (x instanceof UnsignedLong) { - if (y instanceof UnsignedLong) { - return x.equals(y); - } - if (y instanceof Double) { - return compareUintDouble((UnsignedLong) x, (Double) y) == 0; - } - if (y instanceof Long) { - return compareUintInt((UnsignedLong) x, (Long) y) == 0; - } - } - return false; + // Only x needs a NaN check: if y is NaN and x is not, numericCompare returns -1 != 0. + return isNumeric(x) + && isNumeric(y) + && !(x instanceof Double && ((Double) x).isNaN()) + && numericCompare(x, y) == 0; } - /** Compare two numeric values of any type (double, int, uint). */ + /** + * Compares two numeric values of any type (double, int, uint) for sorting (ordering {@code NaN} + * last and equal to itself, with {@code -0.0 == 0.0}); use {@link #numericEquals} for CEL {@code + * ==} semantics. + */ public static int numericCompare(Number x, Number y) { if (x instanceof Double) { if (y instanceof Double) { - return ((Double) x).compareTo((Double) y); + return compareDouble((Double) x, (Double) y); } if (y instanceof Long) { return compareDoubleInt((Double) x, (Long) y); @@ -155,5 +119,10 @@ public static int numericCompare(Number x, Number y) { "Unsupported argument types: " + x.getClass() + ", " + y.getClass()); } + private static int compareDouble(double a, double b) { + // Treat -0.0 and 0.0 as equal via ==; Double.compare orders NaN after all non-NaN numbers. + return a == b ? 0 : Double.compare(a, b); + } + private ComparisonFunctions() {} } diff --git a/common/src/test/java/dev/cel/common/internal/ComparisonFunctionsTest.java b/common/src/test/java/dev/cel/common/internal/ComparisonFunctionsTest.java index 7508d2ed8..dc42f1396 100644 --- a/common/src/test/java/dev/cel/common/internal/ComparisonFunctionsTest.java +++ b/common/src/test/java/dev/cel/common/internal/ComparisonFunctionsTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.primitives.UnsignedLong; +import com.google.testing.junit.testparameterinjector.TestParameter; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import com.google.testing.junit.testparameterinjector.TestParameters; import java.math.BigInteger; @@ -28,10 +29,12 @@ public final class ComparisonFunctionsTest { @Test @TestParameters("{x: 1.0, y: 1, expect: 0}") + @TestParameters("{x: -0.0, y: 0, expect: 0}") @TestParameters("{x: -2.1, y: -2, expect: -1}") @TestParameters("{x: 2.1, y: -3, expect: 1}") @TestParameters("{x: -9223372036854778809.0, y: 1, expect: -1}") @TestParameters("{x: 9223372036854778809.0, y: 1, expect: 1}") + @TestParameters("{x: NaN, y: 0, expect: 1}") public void compareDoubleInt(double x, long y, int expect) { assertThat(ComparisonFunctions.compareDoubleInt(x, y)).isEqualTo(expect); assertThat(ComparisonFunctions.compareIntDouble(y, x)).isEqualTo(-1 * expect); @@ -39,9 +42,11 @@ public void compareDoubleInt(double x, long y, int expect) { @Test @TestParameters("{x: 1.0, y: 1, expect: 0}") + @TestParameters("{x: -0.0, y: 0, expect: 0}") @TestParameters("{x: -2.1, y: 0, expect: -1}") @TestParameters("{x: 2.1, y: 1, expect: 1}") @TestParameters("{x: 18446744073709653620.1, y: 1, expect: 1}") + @TestParameters("{x: NaN, y: 0, expect: 1}") public void compareDoubleUint(double x, long y, int expect) { UnsignedLong uy = UnsignedLong.valueOf(y); assertThat(ComparisonFunctions.compareDoubleUint(x, uy)).isEqualTo(expect); @@ -93,4 +98,44 @@ public void numericCompareUintInt(long x, long y, int expect) { assertThat(ComparisonFunctions.numericCompare(ux, y)).isEqualTo(expect); assertThat(ComparisonFunctions.numericCompare(y, ux)).isEqualTo(-1 * expect); } + + @Test + @TestParameters("{x: -0.0, y: 0.0, expect: 0}") + @TestParameters("{x: 1.0, y: 2.0, expect: -1}") + @TestParameters("{x: NaN, y: 1.0, expect: 1}") + @TestParameters("{x: NaN, y: NaN, expect: 0}") + public void numericCompare_doubles(double x, double y, int expect) { + assertThat(ComparisonFunctions.numericCompare(x, y)).isEqualTo(expect); + assertThat(ComparisonFunctions.numericCompare(y, x)).isEqualTo(-1 * expect); + } + + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum NumericEqualsTestCase { + NEGATIVE_ZERO_AND_POSITIVE_ZERO(-0.0, 0.0, true), + NEGATIVE_ZERO_AND_LONG_ZERO(-0.0, 0L, true), + NEGATIVE_ZERO_AND_ULONG_ZERO(-0.0, UnsignedLong.ZERO, true), + DISTINCT_DOUBLES(1.0, 2.0, false), + NAN_AND_NAN(Double.NaN, Double.NaN, false), + NAN_AND_LONG_ZERO(Double.NaN, 0L, false), + NAN_AND_ULONG_ZERO(Double.NaN, UnsignedLong.ZERO, false), + SHORT_AND_LONG((short) 1, 1L, false); + + private final Number x; + private final Number y; + private final boolean expected; + + NumericEqualsTestCase(Number x, Number y, boolean expected) { + this.x = x; + this.y = y; + this.expected = expected; + } + } + + @Test + public void numericEquals_isSymmetric(@TestParameter NumericEqualsTestCase testCase) { + assertThat(ComparisonFunctions.numericEquals(testCase.x, testCase.y)) + .isEqualTo(testCase.expected); + assertThat(ComparisonFunctions.numericEquals(testCase.y, testCase.x)) + .isEqualTo(testCase.expected); + } } diff --git a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel index db0aea5cc..28e1abbb3 100644 --- a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel +++ b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel @@ -138,56 +138,16 @@ _ALL_TESTS = [ ] _TESTS_TO_SKIP_SPEC_IMPORT = [ - # Skip until duration/timestamp range, negative zero/NaN comparisons, and proto wrapper validation are updated for spec parity. + # Skip until duration/timestamp range, container NaN/negative-zero equality, and proto wrapper validation are updated for spec parity. "comparisons/bound/bound_list_nan_eq_self_false", "comparisons/bound/bound_list_nan_ne_self_true", "comparisons/bound/bound_map_nan_eq_self_false", "comparisons/bound/bound_map_nan_ne_self_true", - "comparisons/eq_literal/eq_double_neg_zero_int", - "comparisons/eq_literal/eq_double_neg_zero_uint", - "comparisons/eq_literal/eq_int_double_neg_zero", - "comparisons/eq_literal/eq_list_mixed_type_neg_zero", - "comparisons/eq_literal/eq_list_mixed_type_neg_zero_uint", - "comparisons/eq_literal/eq_map_mixed_type_neg_zero", - "comparisons/eq_literal/eq_map_mixed_type_neg_zero_uint", - "comparisons/eq_literal/eq_nested_list_mixed_type_neg_zero", - "comparisons/eq_literal/eq_nested_map_list_neg_zero", - "comparisons/eq_literal/eq_uint_double_neg_zero", - "comparisons/gt_literal/not_gt_dyn_double_nan_int", - "comparisons/gt_literal/not_gt_dyn_double_nan_uint", - "comparisons/gt_literal/not_gt_dyn_int_double_neg_zero", - "comparisons/gt_literal/not_gt_dyn_uint_double_neg_zero", - "comparisons/gte_literal/gte_dyn_double_neg_zero_int", - "comparisons/gte_literal/gte_dyn_double_neg_zero_uint", - "comparisons/gte_literal/not_gte_dyn_double_nan_int", - "comparisons/gte_literal/not_gte_dyn_double_nan_uint", - "comparisons/in_list_literal/int_zero_in_list_neg_zero", "comparisons/in_list_literal/nan_not_in_list", - "comparisons/in_list_literal/neg_zero_in_list_int", - "comparisons/in_list_literal/neg_zero_in_list_uint", "comparisons/in_list_literal/nested_list_nan_not_in_list", "comparisons/in_list_literal/nested_list_neg_zero_in_list", "comparisons/in_list_literal/nested_map_nan_not_in_list", "comparisons/in_list_literal/nested_map_neg_zero_in_list", - "comparisons/in_list_literal/uint_zero_in_list_neg_zero", - "comparisons/lt_literal/not_lt_dyn_double_neg_zero_int", - "comparisons/lt_literal/not_lt_dyn_double_neg_zero_uint", - "comparisons/lt_literal/not_lt_dyn_int_double_nan", - "comparisons/lt_literal/not_lt_dyn_uint_double_nan", - "comparisons/lte_literal/lte_dyn_int_double_neg_zero", - "comparisons/lte_literal/lte_dyn_uint_double_neg_zero", - "comparisons/lte_literal/not_lte_dyn_int_double_nan", - "comparisons/lte_literal/not_lte_dyn_uint_double_nan", - "comparisons/ne_literal/not_ne_double_neg_zero_int", - "comparisons/ne_literal/not_ne_double_neg_zero_uint", - "comparisons/ne_literal/not_ne_int_double_neg_zero", - "comparisons/ne_literal/not_ne_list_mixed_type_neg_zero", - "comparisons/ne_literal/not_ne_list_mixed_type_neg_zero_uint", - "comparisons/ne_literal/not_ne_map_mixed_type_neg_zero", - "comparisons/ne_literal/not_ne_map_mixed_type_neg_zero_uint", - "comparisons/ne_literal/not_ne_nested_list_mixed_type_neg_zero", - "comparisons/ne_literal/not_ne_nested_map_list_neg_zero", - "comparisons/ne_literal/not_ne_uint_double_neg_zero", "timestamps/duration_conversions/from_string_max", "timestamps/duration_conversions/from_string_min", "timestamps/duration_conversions/toString_duration_max", diff --git a/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java b/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java index 80d62c9d4..2fe7cb1b4 100644 --- a/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java +++ b/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java @@ -220,6 +220,8 @@ public void range_throws(String expression, String expectedError) throws Excepti "{expression: '[1, 2.0, \"c\", 3, \"c\", 1].distinct()', " + "expected: '[1, 2.0, \"c\", 3]'}") @TestParameters("{expression: '[1, 1.0, 2, 2u].distinct()', expected: '[1, 2]'}") + @TestParameters("{expression: '[-0.0, 0.0, 0, 0u].distinct()', expected: '[-0.0]'}") + @TestParameters("{expression: '[0.0/0.0, 0.0/0.0].distinct()', expected: '[0.0/0.0, 0.0/0.0]'}") @TestParameters("{expression: '[[1], [1], [2]].distinct()', expected: '[[1], [2]]'}") @TestParameters( "{expression: '[SimpleTest{name: \"a\"}, SimpleTest{name: \"b\"}, SimpleTest{name: \"a\"}]" @@ -281,6 +283,9 @@ public void sort_success(String expression, String expected) throws Exception { @Test @TestParameters("{expression: '[3.0, 2, 1u].sort()', expected: '[1u, 2, 3.0]'}") @TestParameters("{expression: '[4, 3, 2, 1].sort()', expected: '[1, 2, 3, 4]'}") + @TestParameters( + "{expression: '[0.0/0.0, 1.0, -0.0, 0.0].sort()', expected: '[-0.0, 0.0, 1.0, 0.0/0.0]'}") + @TestParameters("{expression: '[0, -0.0, 0u].sort()', expected: '[0, -0.0, 0u]'}") public void sort_success_heterogeneousNumbers(String expression, String expected) throws Exception { Object result = eval(cel, expression); diff --git a/extensions/src/test/java/dev/cel/extensions/CelMathExtensionsTest.java b/extensions/src/test/java/dev/cel/extensions/CelMathExtensionsTest.java index 0dae9c846..8e277c0a8 100644 --- a/extensions/src/test/java/dev/cel/extensions/CelMathExtensionsTest.java +++ b/extensions/src/test/java/dev/cel/extensions/CelMathExtensionsTest.java @@ -102,6 +102,7 @@ public void setUp() { @TestParameters( "{expr: 'math.greatest([dyn(5.4), dyn(10), dyn(3u), dyn(-5.0), dyn(3.5)])', expectedResult:" + " 10}") + @TestParameters("{expr: 'math.greatest(0, -0.0)', expectedResult: 0}") public void greatest_intResult_success(String expr, long expectedResult) throws Exception { Object result = eval(expr); @@ -142,6 +143,7 @@ public void greatest_intResult_success(String expr, long expectedResult) throws @TestParameters( "{expr: 'math.greatest([dyn(5.4), dyn(10.0), dyn(3u), dyn(-5.0), dyn(3.5)])', expectedResult:" + " 10.0}") + @TestParameters("{expr: 'math.greatest(-0.0, 0)', expectedResult: -0.0}") public void greatest_doubleResult_success(String expr, double expectedResult) throws Exception { Object result = eval(expr); @@ -319,6 +321,7 @@ public void greatest_nonMathNamespace_success(String expr) throws Exception { @TestParameters( "{expr: 'math.least([dyn(5.4), dyn(-10), dyn(3u), dyn(-5.0), dyn(3.5)])', expectedResult:" + " -10}") + @TestParameters("{expr: 'math.least(0, -0.0)', expectedResult: 0}") public void least_intResult_success(String expr, long expectedResult) throws Exception { Object result = eval(expr); @@ -358,6 +361,7 @@ public void least_intResult_success(String expr, long expectedResult) throws Exc @TestParameters( "{expr: 'math.least([dyn(5.4), dyn(10.0), dyn(3u), dyn(-5.0), dyn(3.5)])', expectedResult:" + " -5.0}") + @TestParameters("{expr: 'math.least(-0.0, 0)', expectedResult: -0.0}") public void least_doubleResult_success(String expr, double expectedResult) throws Exception { Object result = eval(expr); diff --git a/runtime/src/main/java/dev/cel/runtime/standard/GreaterEqualsOperator.java b/runtime/src/main/java/dev/cel/runtime/standard/GreaterEqualsOperator.java index c04b4398f..c57868936 100644 --- a/runtime/src/main/java/dev/cel/runtime/standard/GreaterEqualsOperator.java +++ b/runtime/src/main/java/dev/cel/runtime/standard/GreaterEqualsOperator.java @@ -156,34 +156,35 @@ public enum GreaterEqualsOverload implements CelStandardOverload { UnsignedLong.class, Long.class, (UnsignedLong x, Long y) -> ComparisonFunctions.compareUintInt(x, y) >= 0)), + // Use IEEE 754 primitive comparison so NaN yields false (ComparisonFunctions orders NaN last). GREATER_EQUALS_INT64_DOUBLE( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "greater_equals_int64_double", Long.class, Double.class, - (Long x, Double y) -> ComparisonFunctions.compareIntDouble(x, y) >= 0)), + (Long x, Double y) -> (double) x >= y)), GREATER_EQUALS_DOUBLE_INT64( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "greater_equals_double_int64", Double.class, Long.class, - (Double x, Long y) -> ComparisonFunctions.compareDoubleInt(x, y) >= 0)), + (Double x, Long y) -> x >= (double) y)), GREATER_EQUALS_UINT64_DOUBLE( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "greater_equals_uint64_double", UnsignedLong.class, Double.class, - (UnsignedLong x, Double y) -> ComparisonFunctions.compareUintDouble(x, y) >= 0)), + (UnsignedLong x, Double y) -> x.doubleValue() >= y)), GREATER_EQUALS_DOUBLE_UINT64( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "greater_equals_double_uint64", Double.class, UnsignedLong.class, - (Double x, UnsignedLong y) -> ComparisonFunctions.compareDoubleUint(x, y) >= 0)); + (Double x, UnsignedLong y) -> x >= y.doubleValue())); private final CelStandardOverload standardOverload; diff --git a/runtime/src/main/java/dev/cel/runtime/standard/GreaterOperator.java b/runtime/src/main/java/dev/cel/runtime/standard/GreaterOperator.java index 80edb8a9b..be008f91d 100644 --- a/runtime/src/main/java/dev/cel/runtime/standard/GreaterOperator.java +++ b/runtime/src/main/java/dev/cel/runtime/standard/GreaterOperator.java @@ -148,34 +148,35 @@ public enum GreaterOverload implements CelStandardOverload { UnsignedLong.class, Long.class, (UnsignedLong x, Long y) -> ComparisonFunctions.compareUintInt(x, y) == 1)), + // Use IEEE 754 primitive comparison so NaN yields false (ComparisonFunctions orders NaN last). GREATER_INT64_DOUBLE( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "greater_int64_double", Long.class, Double.class, - (Long x, Double y) -> ComparisonFunctions.compareIntDouble(x, y) == 1)), + (Long x, Double y) -> (double) x > y)), GREATER_DOUBLE_INT64( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "greater_double_int64", Double.class, Long.class, - (Double x, Long y) -> ComparisonFunctions.compareDoubleInt(x, y) == 1)), + (Double x, Long y) -> x > (double) y)), GREATER_UINT64_DOUBLE( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "greater_uint64_double", UnsignedLong.class, Double.class, - (UnsignedLong x, Double y) -> ComparisonFunctions.compareUintDouble(x, y) == 1)), + (UnsignedLong x, Double y) -> x.doubleValue() > y)), GREATER_DOUBLE_UINT64( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "greater_double_uint64", Double.class, UnsignedLong.class, - (Double x, UnsignedLong y) -> ComparisonFunctions.compareDoubleUint(x, y) == 1)), + (Double x, UnsignedLong y) -> x > y.doubleValue())), ; private final CelStandardOverload standardOverload; diff --git a/runtime/src/main/java/dev/cel/runtime/standard/LessEqualsOperator.java b/runtime/src/main/java/dev/cel/runtime/standard/LessEqualsOperator.java index 7688acbc1..56a0b6101 100644 --- a/runtime/src/main/java/dev/cel/runtime/standard/LessEqualsOperator.java +++ b/runtime/src/main/java/dev/cel/runtime/standard/LessEqualsOperator.java @@ -152,34 +152,35 @@ public enum LessEqualsOverload implements CelStandardOverload { UnsignedLong.class, Long.class, (UnsignedLong x, Long y) -> ComparisonFunctions.compareUintInt(x, y) <= 0)), + // Use IEEE 754 primitive comparison so NaN yields false (ComparisonFunctions orders NaN last). LESS_EQUALS_INT64_DOUBLE( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "less_equals_int64_double", Long.class, Double.class, - (Long x, Double y) -> ComparisonFunctions.compareIntDouble(x, y) <= 0)), + (Long x, Double y) -> (double) x <= y)), LESS_EQUALS_DOUBLE_INT64( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "less_equals_double_int64", Double.class, Long.class, - (Double x, Long y) -> ComparisonFunctions.compareDoubleInt(x, y) <= 0)), + (Double x, Long y) -> x <= (double) y)), LESS_EQUALS_UINT64_DOUBLE( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "less_equals_uint64_double", UnsignedLong.class, Double.class, - (UnsignedLong x, Double y) -> ComparisonFunctions.compareUintDouble(x, y) <= 0)), + (UnsignedLong x, Double y) -> x.doubleValue() <= y)), LESS_EQUALS_DOUBLE_UINT64( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "less_equals_double_uint64", Double.class, UnsignedLong.class, - (Double x, UnsignedLong y) -> ComparisonFunctions.compareDoubleUint(x, y) <= 0)), + (Double x, UnsignedLong y) -> x <= y.doubleValue())), ; private final CelStandardOverload standardOverload; diff --git a/runtime/src/main/java/dev/cel/runtime/standard/LessOperator.java b/runtime/src/main/java/dev/cel/runtime/standard/LessOperator.java index a53a13a92..a926dcc2b 100644 --- a/runtime/src/main/java/dev/cel/runtime/standard/LessOperator.java +++ b/runtime/src/main/java/dev/cel/runtime/standard/LessOperator.java @@ -95,13 +95,14 @@ public enum LessOverload implements CelStandardOverload { (celOptions, runtimeEquality) -> CelFunctionBinding.from( "less_double", Double.class, Double.class, (Double x, Double y) -> x < y)), + // Use IEEE 754 primitive comparison so NaN yields false (ComparisonFunctions orders NaN last). LESS_DOUBLE_UINT64( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "less_double_uint64", Double.class, UnsignedLong.class, - (Double x, UnsignedLong y) -> ComparisonFunctions.compareDoubleUint(x, y) == -1)), + (Double x, UnsignedLong y) -> x < y.doubleValue())), LESS_INT64_UINT64( (celOptions, runtimeEquality) -> CelFunctionBinding.from( @@ -122,21 +123,21 @@ public enum LessOverload implements CelStandardOverload { "less_int64_double", Long.class, Double.class, - (Long x, Double y) -> ComparisonFunctions.compareIntDouble(x, y) == -1)), + (Long x, Double y) -> (double) x < y)), LESS_DOUBLE_INT64( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "less_double_int64", Double.class, Long.class, - (Double x, Long y) -> ComparisonFunctions.compareDoubleInt(x, y) == -1)), + (Double x, Long y) -> x < (double) y)), LESS_UINT64_DOUBLE( (celOptions, runtimeEquality) -> CelFunctionBinding.from( "less_uint64_double", UnsignedLong.class, Double.class, - (UnsignedLong x, Double y) -> ComparisonFunctions.compareUintDouble(x, y) == -1)), + (UnsignedLong x, Double y) -> x.doubleValue() < y)), LESS_DURATION( (celOptions, runtimeEquality) -> { if (celOptions.evaluateCanonicalTypesToNativeValues()) { diff --git a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java index b2f8c68e3..2cb50e4ce 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java @@ -102,7 +102,8 @@ @RunWith(TestParameterInjector.class) public final class ProgramPlannerTest { // Note that the following deps will be built from top-level builder APIs - private static final CelOptions CEL_OPTIONS = CelOptions.current().build(); + private static final CelOptions CEL_OPTIONS = + CelOptions.current().enableHeterogeneousNumericComparisons(true).build(); private static final CelTypeProvider TYPE_PROVIDER = new CombinedCelTypeProvider( DefaultTypeProvider.getInstance(), @@ -179,6 +180,7 @@ public final class ProgramPlannerTest { private static final CelCompiler CEL_COMPILER = CelCompilerFactory.standardCelCompilerBuilder() + .setOptions(CEL_OPTIONS) .setStandardMacros(CelStandardMacro.STANDARD_MACROS) .addMacros(COMPREHENSION_TEST_MACROS) .addFunctionDeclarations( @@ -235,6 +237,7 @@ private static DefaultDispatcher newDispatcher(CelOptions celOptions) { StandardFunction.GREATER, StandardFunction.GREATER_EQUALS, StandardFunction.LESS, + StandardFunction.LESS_EQUALS, StandardFunction.DIVIDE, StandardFunction.EQUALS, StandardFunction.NOT_STRICTLY_FALSE, @@ -2726,6 +2729,110 @@ public void plan_call_matches_regexProgramSizeExceedsLimit_throws( + ", Provided: 7"); } + @Test + @TestParameters("{expression: '-1 < 0.0', expectedResult: true}") + @TestParameters("{expression: '1 < 0.0', expectedResult: false}") + @TestParameters("{expression: '-1.0 < 0', expectedResult: true}") + @TestParameters("{expression: '1.0 < 0', expectedResult: false}") + @TestParameters("{expression: '0u < 1.0', expectedResult: true}") + @TestParameters("{expression: '1u < 0.0', expectedResult: false}") + @TestParameters("{expression: '-1.0 < 0u', expectedResult: true}") + @TestParameters("{expression: '1.0 < 0u', expectedResult: false}") + @TestParameters("{expression: '0 < -0.0', expectedResult: false}") + @TestParameters("{expression: '-0.0 < 0', expectedResult: false}") + @TestParameters("{expression: '0u < -0.0', expectedResult: false}") + @TestParameters("{expression: '-0.0 < 0u', expectedResult: false}") + @TestParameters("{expression: '1 < 0.0 / 0.0', expectedResult: false}") + @TestParameters("{expression: '0.0 / 0.0 < 1', expectedResult: false}") + @TestParameters("{expression: '1u < 0.0 / 0.0', expectedResult: false}") + @TestParameters("{expression: '0.0 / 0.0 < 1u', expectedResult: false}") + public void plan_call_less_heterogeneousNumeric(String expression, boolean expectedResult) + throws Exception { + Program program = PLANNER.plan(compile(expression)); + + Object result = program.eval(); + + assertThat(result).isEqualTo(expectedResult); + } + + @Test + @TestParameters("{expression: '-1 <= 0.0', expectedResult: true}") + @TestParameters("{expression: '1 <= 0.0', expectedResult: false}") + @TestParameters("{expression: '-1.0 <= 0', expectedResult: true}") + @TestParameters("{expression: '1.0 <= 0', expectedResult: false}") + @TestParameters("{expression: '0u <= 1.0', expectedResult: true}") + @TestParameters("{expression: '1u <= 0.0', expectedResult: false}") + @TestParameters("{expression: '-1.0 <= 0u', expectedResult: true}") + @TestParameters("{expression: '1.0 <= 0u', expectedResult: false}") + @TestParameters("{expression: '0 <= -0.0', expectedResult: true}") + @TestParameters("{expression: '-0.0 <= 0', expectedResult: true}") + @TestParameters("{expression: '0u <= -0.0', expectedResult: true}") + @TestParameters("{expression: '-0.0 <= 0u', expectedResult: true}") + @TestParameters("{expression: '1 <= 0.0 / 0.0', expectedResult: false}") + @TestParameters("{expression: '0.0 / 0.0 <= 1', expectedResult: false}") + @TestParameters("{expression: '1u <= 0.0 / 0.0', expectedResult: false}") + @TestParameters("{expression: '0.0 / 0.0 <= 1u', expectedResult: false}") + public void plan_call_lessEquals_heterogeneousNumeric(String expression, boolean expectedResult) + throws Exception { + Program program = PLANNER.plan(compile(expression)); + + Object result = program.eval(); + + assertThat(result).isEqualTo(expectedResult); + } + + @Test + @TestParameters("{expression: '-1 > 0.0', expectedResult: false}") + @TestParameters("{expression: '1 > 0.0', expectedResult: true}") + @TestParameters("{expression: '-1.0 > 0', expectedResult: false}") + @TestParameters("{expression: '1.0 > 0', expectedResult: true}") + @TestParameters("{expression: '0u > 1.0', expectedResult: false}") + @TestParameters("{expression: '1u > 0.0', expectedResult: true}") + @TestParameters("{expression: '-1.0 > 0u', expectedResult: false}") + @TestParameters("{expression: '1.0 > 0u', expectedResult: true}") + @TestParameters("{expression: '0 > -0.0', expectedResult: false}") + @TestParameters("{expression: '-0.0 > 0', expectedResult: false}") + @TestParameters("{expression: '0u > -0.0', expectedResult: false}") + @TestParameters("{expression: '-0.0 > 0u', expectedResult: false}") + @TestParameters("{expression: '1 > 0.0 / 0.0', expectedResult: false}") + @TestParameters("{expression: '0.0 / 0.0 > 1', expectedResult: false}") + @TestParameters("{expression: '1u > 0.0 / 0.0', expectedResult: false}") + @TestParameters("{expression: '0.0 / 0.0 > 1u', expectedResult: false}") + public void plan_call_greater_heterogeneousNumeric(String expression, boolean expectedResult) + throws Exception { + Program program = PLANNER.plan(compile(expression)); + + Object result = program.eval(); + + assertThat(result).isEqualTo(expectedResult); + } + + @Test + @TestParameters("{expression: '-1 >= 0.0', expectedResult: false}") + @TestParameters("{expression: '1 >= 0.0', expectedResult: true}") + @TestParameters("{expression: '-1.0 >= 0', expectedResult: false}") + @TestParameters("{expression: '1.0 >= 0', expectedResult: true}") + @TestParameters("{expression: '0u >= 1.0', expectedResult: false}") + @TestParameters("{expression: '1u >= 0.0', expectedResult: true}") + @TestParameters("{expression: '-1.0 >= 0u', expectedResult: false}") + @TestParameters("{expression: '1.0 >= 0u', expectedResult: true}") + @TestParameters("{expression: '0 >= -0.0', expectedResult: true}") + @TestParameters("{expression: '-0.0 >= 0', expectedResult: true}") + @TestParameters("{expression: '0u >= -0.0', expectedResult: true}") + @TestParameters("{expression: '-0.0 >= 0u', expectedResult: true}") + @TestParameters("{expression: '1 >= 0.0 / 0.0', expectedResult: false}") + @TestParameters("{expression: '0.0 / 0.0 >= 1', expectedResult: false}") + @TestParameters("{expression: '1u >= 0.0 / 0.0', expectedResult: false}") + @TestParameters("{expression: '0.0 / 0.0 >= 1u', expectedResult: false}") + public void plan_call_greaterEquals_heterogeneousNumeric( + String expression, boolean expectedResult) throws Exception { + Program program = PLANNER.plan(compile(expression)); + + Object result = program.eval(); + + assertThat(result).isEqualTo(expectedResult); + } + private static ProgramPlanner newPlannerWithOptions(CelOptions options) { return ProgramPlanner.newPlanner( TYPE_PROVIDER,