From 4d00593223f7373eeda605353180fed78d2067f9 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Wed, 1 Apr 2026 15:28:25 -0700 Subject: [PATCH] Fix partial evaluation to properly check for comprehension bound variables for planner PiperOrigin-RevId: 893142702 --- .../dev/cel/runtime/planner/ActivationWrapper.java | 3 +++ .../java/dev/cel/runtime/planner/EvalFold.java | 5 +++++ .../cel/runtime/planner/NamespacedAttribute.java | 13 ++++++++++++- .../dev/cel/runtime/PlannerInterpreterTest.java | 5 +++++ .../planner_unknownResultSet_success.baseline | 14 +++++++++++++- testing/src/main/java/dev/cel/testing/BUILD.bazel | 1 + .../java/dev/cel/testing/BaseInterpreterTest.java | 3 ++- 7 files changed, 41 insertions(+), 3 deletions(-) diff --git a/runtime/src/main/java/dev/cel/runtime/planner/ActivationWrapper.java b/runtime/src/main/java/dev/cel/runtime/planner/ActivationWrapper.java index f844ab232..8883ac12c 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/ActivationWrapper.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/ActivationWrapper.java @@ -19,4 +19,7 @@ /** Identifies a resolver that can be unwrapped to bypass local variable state. */ public interface ActivationWrapper extends GlobalResolver { GlobalResolver unwrap(); + + /** Returns true if the given name is bound by this local activation wrapper. */ + boolean isLocallyBound(String name); } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java index 197db42ad..2631bf0b9 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java @@ -175,6 +175,11 @@ public GlobalResolver unwrap() { return resolver; } + @Override + public boolean isLocallyBound(String name) { + return name.equals(accuVar) || name.equals(iterVar) || name.equals(iterVar2); + } + @Override public @Nullable Object resolve(String name) { if (name.equals(accuVar)) { diff --git a/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java b/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java index d51336d80..0000ad764 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java @@ -75,7 +75,7 @@ public Object resolve(long exprId, GlobalResolver ctx, ExecutionFrame frame) { PartialVars partialVars = frame.partialVars().orElse(null); - if (partialVars != null) { + if (partialVars != null && !isLocallyBound(resolver, name)) { ImmutableList patterns = partialVars.unknowns(); // Avoid enhanced for loop to prevent UnmodifiableIterator from being allocated for (int i = 0; i < qualifiers.size(); i++) { @@ -151,6 +151,17 @@ private static Long getEnumValue(EnumType enumType, String field) { String.format("Field %s was not found on enum %s", enumType.name(), field))); } + private boolean isLocallyBound(GlobalResolver resolver, String name) { + while (resolver instanceof ActivationWrapper) { + ActivationWrapper wrapper = (ActivationWrapper) resolver; + if (wrapper.isLocallyBound(name)) { + return true; + } + resolver = wrapper.unwrap(); + } + return false; + } + private GlobalResolver unwrapToNonLocal(GlobalResolver resolver) { while (resolver instanceof ActivationWrapper) { resolver = ((ActivationWrapper) resolver).unwrap(); diff --git a/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java b/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java index 181842ab4..2b0e53298 100644 --- a/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java +++ b/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java @@ -283,6 +283,11 @@ public void planner_unknownResultSet_success() { declareVariable("unknown_list", ListType.create(SimpleType.INT)); source = "unknown_list.map(x, x)"; runTest(variables, CelAttributePattern.fromQualifiedIdentifier("unknown_list")); + + clearAllDeclarations(); + declareVariable("x", StructTypeReference.create(TestAllTypes.getDescriptor().getFullName())); + source = "cel.bind(x, [1, 2, 3], 1 in x)"; + runTest(variables, CelAttributePattern.fromQualifiedIdentifier("x")); } @Test diff --git a/runtime/src/test/resources/planner_unknownResultSet_success.baseline b/runtime/src/test/resources/planner_unknownResultSet_success.baseline index 2f2c218d0..c5e8867db 100644 --- a/runtime/src/test/resources/planner_unknownResultSet_success.baseline +++ b/runtime/src/test/resources/planner_unknownResultSet_success.baseline @@ -458,4 +458,16 @@ single_timestamp { seconds: 15 } , unknown_attributes=[unknown_list]} -result: CelUnknownSet{attributes=[unknown_list], unknownExprIds=[1]} \ No newline at end of file +result: CelUnknownSet{attributes=[unknown_list], unknownExprIds=[1]} + +Source: cel.bind(x, [1, 2, 3], 1 in x) +declare x { + value cel.expr.conformance.proto3.TestAllTypes +} +=====> +bindings: {x=single_string: "test" +single_timestamp { + seconds: 15 +} +, unknown_attributes=[x]} +result: true \ No newline at end of file diff --git a/testing/src/main/java/dev/cel/testing/BUILD.bazel b/testing/src/main/java/dev/cel/testing/BUILD.bazel index 2ecabdf05..5ee142200 100644 --- a/testing/src/main/java/dev/cel/testing/BUILD.bazel +++ b/testing/src/main/java/dev/cel/testing/BUILD.bazel @@ -87,6 +87,7 @@ java_library( "//common/types:message_type_provider", "//common/types:type_providers", "//common/values:cel_byte_string", + "//extensions", "//extensions:optional_library", "//runtime", "//runtime:function_binding", diff --git a/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java b/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java index 42cb5e41b..bda56a19e 100644 --- a/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java +++ b/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java @@ -74,6 +74,7 @@ import dev.cel.expr.conformance.proto3.TestAllTypes; import dev.cel.expr.conformance.proto3.TestAllTypes.NestedEnum; import dev.cel.expr.conformance.proto3.TestAllTypes.NestedMessage; +import dev.cel.extensions.CelExtensions; import dev.cel.extensions.CelOptionalLibrary; import dev.cel.runtime.CelAttributePattern; import dev.cel.runtime.CelEvaluationException; @@ -153,7 +154,7 @@ protected void prepareCompiler(CelTypeProvider typeProvider) { this.celCompiler = celCompiler .toCompilerBuilder() - .addLibraries(CelOptionalLibrary.INSTANCE) + .addLibraries(CelOptionalLibrary.INSTANCE, CelExtensions.bindings()) .setOptions(celOptions) .build(); }