fix(interpreter): propagate unknown/error operands through == and !=#865
Open
clmatt wants to merge 1 commit into
Open
fix(interpreter): propagate unknown/error operands through == and !=#865clmatt wants to merge 1 commit into
clmatt wants to merge 1 commit into
Conversation
|
|
dimas-b
previously approved these changes
Jun 9, 2026
| void equalityWithUnknownOperandStaysUnknown() { | ||
| assertThat(evalWithUnknownStringX("x == 'foo'")).isInstanceOf(UnknownT.class); | ||
| assertThat(evalWithUnknownStringX("x != 'foo'")).isInstanceOf(UnknownT.class); | ||
| assertThat(evalWithUnknownStringX("false || x == 'foo'")).isInstanceOf(UnknownT.class); |
Member
There was a problem hiding this comment.
would it make sense to add a test case for false && x == 'foo' -> false?.. or is it covered elsewhere?
EvalEq and EvalNe evaluated their operands and called lhs.equal(rhs) directly, without first checking whether either operand was unknown or error. Because UnknownT.equal(...) returns BoolT.False, any "==" against an unknown collapsed to a concrete false (and "!=" to a concrete true) during partial evaluation, instead of propagating the unknown. The sibling EvalBinary (used by <, <=, >, >=) already guards with isUnknownOrError, and cel-go guards its equality evaluators the same way. Add the same guard to EvalEq and EvalNe so an undecidable equality is deferred rather than resolved. This fixes residual-AST / partial evaluation: a policy of the form `... && claims.email == "x"` evaluated with `email` unknown now yields a residual of the deferred comparison instead of collapsing the whole expression to false. ResidualAst_Complex is updated to assert the corrected (unknown-propagating) outcome, and a focused regression test, InterpreterTest.equalityWithUnknownOperandStaysUnknown, is added.
clmatt
force-pushed
the
fix/eq-ne-propagate-unknown
branch
from
b235d57 to
85ab10f
Compare
dimas-b
approved these changes
Jun 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Using partial evaluate with unknown variables with
==or!=would incorrectly outputtrueorfalseas opposed tounknown.See the issue at #863.
For the corrected test, email is unknown, so email == "wiley@acme.co" (and therefore the whole expression) should evaluate to unknown, not false.