Skip to content

Align heterogeneous equality with CEL spec #903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
snazy merged 1 commit into from
Jul 17, 2026
Merged

Align heterogeneous equality with CEL spec #903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions core/src/main/java/org/projectnessie/cel/common/types/BoolT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ public Val equal(Val other) {
case Bool:
return Types.boolOf(b == ((BoolT) other).b);
case Null:
case Bytes:
case Double:
case Int:
case List:
case Map:
case Object:
case String:
case Type:
case Uint:
return False;
default:
return noSuchOverload(this, "equal", other);
Expand Down
9 changes: 9 additions & 0 deletions core/src/main/java/org/projectnessie/cel/common/types/BytesT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,15 @@ public Val equal(Val other) {
case Bytes:
return boolOf(Arrays.equals(b, ((BytesT) other).b));
case Null:
case Bool:
case Double:
case Int:
case List:
case Map:
case Object:
case String:
case Type:
case Uint:
return False;
default:
return noSuchOverload(this, "equal", other);
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/java/org/projectnessie/cel/common/types/DoubleT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ public Val equal(Val other) {
case Uint:
case Int:
case Double:
case String:
Val converted = other.convertToType(type());
if (converted.type().typeEnum() == TypeEnum.Err) {
return converted;
Expand All @@ -208,9 +207,13 @@ public Val equal(Val other) {
// TODO: Handle NaNs properly.
return boolOf(d == o);
case Null:
case Bool:
case Bytes:
case List:
case Map:
case Object:
case String:
case Type:
return False;
default:
return noSuchOverload(this, "equal", other);
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/java/org/projectnessie/cel/common/types/IntT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,16 +240,19 @@ public Val equal(Val other) {
return False;
}
case Int:
case String:
Val converted = other.convertToType(type());
if (converted.type().typeEnum() == TypeEnum.Err) {
return converted;
}
return boolOf(i == converted.intValue());
case Null:
case Bool:
case Bytes:
case List:
case Map:
case Object:
case String:
case Type:
return False;
default:
return noSuchOverload(this, "equal", other);
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/projectnessie/cel/common/types/NullT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ public Val equal(Val other) {
case Double:
case String:
case Bytes:
case Bool:
case List:
case Map:
case Object:
case Type:
return False;
default:
return noSuchOverload(this, "equal", other);
Expand Down
10 changes: 7 additions & 3 deletions core/src/main/java/org/projectnessie/cel/common/types/StringT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,16 @@ public Val equal(Val other) {
switch (other.type().typeEnum()) {
case String:
return boolOf(s.equals(((StringT) other).s));
case Null:
case Bool:
case Bytes:
case Int:
case Uint:
case Double:
case Bool:
return boolOf(s.equals(((StringT) other.convertToType(StringType)).s));
case Null:
case List:
case Map:
case Object:
case Type:
return False;
default:
return noSuchOverload(this, "equal", other);
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/projectnessie/cel/common/types/TypeT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
*/
package org.projectnessie.cel.common.types;

import static org.projectnessie.cel.common.types.BoolT.False;
import static org.projectnessie.cel.common.types.Err.newTypeConversionError;
import static org.projectnessie.cel.common.types.Err.noSuchOverload;
import static org.projectnessie.cel.common.types.StringT.stringOf;
import static org.projectnessie.cel.common.types.Types.boolOf;

Expand Down Expand Up @@ -106,7 +106,7 @@ public Val convertToType(Type typeVal) {
@Override
public Val equal(Val other) {
if (TypeType != other.type()) {
return noSuchOverload(this, "equal", other);
return False;
}
return boolOf(this.equals(other));
}
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/java/org/projectnessie/cel/common/types/UintT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,16 +223,19 @@ public Val equal(Val other) {
case Double:
return other.equal(this);
case Uint:
case String:
Val converted = other.convertToType(type());
if (converted.type().typeEnum() == TypeEnum.Err) {
return converted;
}
return boolOf(i == converted.intValue());
case Null:
case Bool:
case Bytes:
case List:
case Map:
case Object:
case String:
case Type:
return False;
default:
return noSuchOverload(this, "equal", other);
Expand Down
10 changes: 2 additions & 8 deletions core/src/test/java/org/projectnessie/cel/common/types/BoolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,8 @@ void boolConvertToType() {
void boolEqual() {
assertThat(True.equal(True)).extracting(Val::booleanValue).isEqualTo(Boolean.TRUE);
assertThat(False.equal(True)).extracting(Val::booleanValue).isEqualTo(Boolean.FALSE);
assertThat(doubleOf(0.0d).equal(False))
.isInstanceOf(Err.class)
.extracting(Object::toString)
.isEqualTo("no such overload: double.equal(bool)");
assertThat(False.equal(doubleOf(0.0d)))
.isInstanceOf(Err.class)
.extracting(Object::toString)
.isEqualTo("no such overload: bool.equal(double)");
assertThat(doubleOf(0.0d).equal(False)).isSameAs(False);
assertThat(False.equal(doubleOf(0.0d))).isSameAs(False);
}

@Test
Expand Down
4 changes: 2 additions & 2 deletions core/src/test/java/org/projectnessie/cel/common/types/DoubleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,15 +172,15 @@ void doubleDivide() {

@Test
void doubleEqual() {
assertThat(doubleOf(0).equal(False)).matches(Err::isError);
assertThat(doubleOf(0).equal(False)).isSameAs(False);

@XN137 XN137 Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am no expert on the CEL spec, so just wondering what you think about:

Type-checking asserts that arguments to equality operators must be the same type. If the argument types differ, the type-checker will raise an error. However, if at least one argument is dynamically typed, the type-checker considers all arguments dynamic and defers type-agreement checks to the interpreter.

https://github.com/cel-expr/cel-spec/blob/master/doc/langdef.md#equality

would it be preferable to throw an error for equal on incompatible types?
or is this the "dynamically typed" scenario?

@snazy snazy Jul 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CEL-spec just say "those are not equal". Mean, that's the same as Java "foo".equals(1) == false

assertThat(doubleOf(0).equal(NullValue)).isSameAs(False);
assertThat(doubleOf(0).equal(intOf(0))).isSameAs(True);
assertThat(doubleOf(0).equal(intOf(1))).isSameAs(False);
assertThat(doubleOf(0).equal(uintOf(0))).isSameAs(True);
assertThat(doubleOf(0).equal(uintOf(1))).isSameAs(False);
assertThat(doubleOf(0).equal(doubleOf(0))).isSameAs(True);
assertThat(doubleOf(0).equal(doubleOf(1))).isSameAs(False);
assertThat(doubleOf(0).equal(stringOf("0"))).isSameAs(True);
assertThat(doubleOf(0).equal(stringOf("0"))).isSameAs(False);
assertThat(doubleOf(0).equal(stringOf("1"))).isSameAs(False);
}

Expand Down
8 changes: 2 additions & 6 deletions core/src/test/java/org/projectnessie/cel/common/types/IntTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,8 @@ void intDivide() {

@Test
void intEqual() {
assertThat(intOf(0).equal(False))
.isInstanceOf(Err.class)
.extracting(Object::toString)
.isEqualTo("no such overload: int.equal(bool)");
assertThat(intOf(0).equal(False)).isSameAs(False);
assertThat(intOf(0).equal(NullValue)).isSameAs(False);
assertThat(intOf(0).equal(stringOf("0"))).isSameAs(True);
assertThat(intOf(0).equal(stringOf("1"))).isSameAs(False);
assertThat(intOf(0).equal(intOf(0))).isSameAs(True);
assertThat(intOf(0).equal(intOf(1))).isSameAs(False);
Expand All @@ -198,7 +194,7 @@ void intEqual() {
assertThat(intOf(0xffffffffffffffffL).equal(uintOf(0xffffffffffffffffL))).isSameAs(False);
assertThat(intOf(0).equal(doubleOf(0))).isSameAs(True);
assertThat(intOf(0).equal(doubleOf(1))).isSameAs(False);
assertThat(intOf(0).equal(stringOf("0"))).isSameAs(True);
assertThat(intOf(0).equal(stringOf("0"))).isSameAs(False);
assertThat(intOf(0).equal(stringOf("1"))).isSameAs(False);
}

Expand Down
2 changes: 2 additions & 0 deletions core/src/test/java/org/projectnessie/cel/common/types/StringTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ void stringEqual() {
assertThat(stringOf("hello").equal(stringOf("hello"))).isSameAs(True);
assertThat(stringOf("hello").equal(stringOf("hell"))).isSameAs(False);
assertThat(stringOf("c").equal(intOf(99))).isSameAs(False);
assertThat(stringOf("99").equal(intOf(99))).isSameAs(False);
assertThat(stringOf("true").equal(True)).isSameAs(False);
}

@Test
Expand Down
7 changes: 2 additions & 5 deletions core/src/test/java/org/projectnessie/cel/common/types/UintTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,7 @@ void uintDivide() {

@Test
void uintEqual() {
assertThat(uintOf(0).equal(False))
.isInstanceOf(Err.class)
.extracting(Object::toString)
.isEqualTo("no such overload: uint.equal(bool)");
assertThat(uintOf(0).equal(False)).isSameAs(False);
assertThat(uintOf(0).equal(NullValue)).isSameAs(False);
assertThat(uintOf(0).equal(intOf(0))).isSameAs(True);
assertThat(uintOf(0).equal(intOf(1))).isSameAs(False);
Expand All @@ -189,7 +186,7 @@ void uintEqual() {
assertThat(uintOf(0).equal(uintOf(1))).isSameAs(False);
assertThat(uintOf(0).equal(doubleOf(0))).isSameAs(True);
assertThat(uintOf(0).equal(doubleOf(1))).isSameAs(False);
assertThat(uintOf(0).equal(stringOf("0"))).isSameAs(True);
assertThat(uintOf(0).equal(stringOf("0"))).isSameAs(False);

@XN137 XN137 Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isnt it surprising that conformance tests did not cover cases like this?
(i am assuming all are passing, or does this change reduce the number of failing conformance tests?)

@snazy snazy Jul 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. I suspect, the conformance tests still use the "old" CEL-spec, where this was allowed.

assertThat(uintOf(0).equal(stringOf("1"))).isSameAs(False);
}

Expand Down
8 changes: 8 additions & 0 deletions core/src/test/java/org/projectnessie/cel/interpreter/InterpreterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,14 @@ static TestCase[] testCases() {
new TestCase(InterpreterTestCase.map_key_mixed_numbers_lossy_double_key)
.expr("{1u: 1.0, 2: 2.0, 3u: 3.0}[3.1]")
.err("no such key: double{3.1}"),
new TestCase(InterpreterTestCase.map_key_string_and_int_are_distinct)
.expr("{1: 'int', '1': 'string'}['1']")

@XN137 XN137 Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have a test that covers trying to use two equal keys in the same map literal?
did this used to fail or had "last one wins" semantics?

@snazy snazy Jul 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have a test that covers trying to use two equal keys in the same map literal?

Yes, for equal heterogeneous numeric keys (somewhere below .expr("{0: 1, 0u: 2}[0.0]"). For exact duplicate same-type keys, no.

did this used to fail or had "last one wins" semantics?

Honestly, I don't know.

.out(stringOf("string")),
new TestCase(InterpreterTestCase.eq_dyn_string_int).expr("dyn('1') == dyn(1)").out(False),
new TestCase(InterpreterTestCase.eq_dyn_int_string).expr("dyn(1) == dyn('1')").out(False),
new TestCase(InterpreterTestCase.eq_dyn_string_bool)
.expr("dyn('true') == dyn(true)")
.out(False),
new TestCase(InterpreterTestCase.zero_based_double_error)
.expr("[7, 8, 9][dyn(0.1)]")
.err("invalid_argument"),
Expand Down
4 changes: 4 additions & 0 deletions core/src/test/java/org/projectnessie/cel/interpreter/InterpreterTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ public enum InterpreterTestCase {
lt_dyn_uint_big_double,
lt_ne_dyn_int_double,
map_key_mixed_numbers_lossy_double_key,
map_key_string_and_int_are_distinct,
eq_dyn_string_int,
eq_dyn_int_string,
eq_dyn_string_bool,
map_value_repeat_key_heterogeneous,
timestamp_eq_timestamp,
timestamp_ne_timestamp,
Expand Down
Loading