Skip to content

Commit b20101e

Browse files
l46kokcopybara-github
authored andcommitted
Fix partial evaluation to properly check for comprehension bound variables for planner
PiperOrigin-RevId: 892757311
1 parent 8e9f1ac commit b20101e

File tree

7 files changed

+41
-3
lines changed

7 files changed

+41
-3
lines changed

runtime/src/main/java/dev/cel/runtime/planner/ActivationWrapper.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,7 @@
1919
/** Identifies a resolver that can be unwrapped to bypass local variable state. */
2020
public interface ActivationWrapper extends GlobalResolver {
2121
GlobalResolver unwrap();
22+
23+
/** Returns true if the given name is bound by this local activation wrapper. */
24+
boolean isLocallyBound(String name);
2225
}

runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,11 @@ public GlobalResolver unwrap() {
175175
return resolver;
176176
}
177177

178+
@Override
179+
public boolean isLocallyBound(String name) {
180+
return name.equals(accuVar) || name.equals(iterVar) || name.equals(iterVar2);
181+
}
182+
178183
@Override
179184
public @Nullable Object resolve(String name) {
180185
if (name.equals(accuVar)) {

runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public Object resolve(long exprId, GlobalResolver ctx, ExecutionFrame frame) {
7575

7676
PartialVars partialVars = frame.partialVars().orElse(null);
7777

78-
if (partialVars != null) {
78+
if (partialVars != null && !isLocallyBound(resolver, name)) {
7979
ImmutableList<CelAttributePattern> patterns = partialVars.unknowns();
8080
// Avoid enhanced for loop to prevent UnmodifiableIterator from being allocated
8181
for (int i = 0; i < qualifiers.size(); i++) {
@@ -151,6 +151,17 @@ private static Long getEnumValue(EnumType enumType, String field) {
151151
String.format("Field %s was not found on enum %s", enumType.name(), field)));
152152
}
153153

154+
private boolean isLocallyBound(GlobalResolver resolver, String name) {
155+
while (resolver instanceof ActivationWrapper) {
156+
ActivationWrapper wrapper = (ActivationWrapper) resolver;
157+
if (wrapper.isLocallyBound(name)) {
158+
return true;
159+
}
160+
resolver = wrapper.unwrap();
161+
}
162+
return false;
163+
}
164+
154165
private GlobalResolver unwrapToNonLocal(GlobalResolver resolver) {
155166
while (resolver instanceof ActivationWrapper) {
156167
resolver = ((ActivationWrapper) resolver).unwrap();

runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,11 @@ public void planner_unknownResultSet_success() {
283283
declareVariable("unknown_list", ListType.create(SimpleType.INT));
284284
source = "unknown_list.map(x, x)";
285285
runTest(variables, CelAttributePattern.fromQualifiedIdentifier("unknown_list"));
286+
287+
clearAllDeclarations();
288+
declareVariable("x", StructTypeReference.create(TestAllTypes.getDescriptor().getFullName()));
289+
source = "cel.bind(x, [1, 2, 3], 1 in x)";
290+
runTest(variables, CelAttributePattern.fromQualifiedIdentifier("x"));
286291
}
287292

288293
@Test

runtime/src/test/resources/planner_unknownResultSet_success.baseline

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,4 +458,16 @@ single_timestamp {
458458
seconds: 15
459459
}
460460
, unknown_attributes=[unknown_list]}
461-
result: CelUnknownSet{attributes=[unknown_list], unknownExprIds=[1]}
461+
result: CelUnknownSet{attributes=[unknown_list], unknownExprIds=[1]}
462+
463+
Source: cel.bind(x, [1, 2, 3], 1 in x)
464+
declare x {
465+
value cel.expr.conformance.proto3.TestAllTypes
466+
}
467+
=====>
468+
bindings: {x=single_string: "test"
469+
single_timestamp {
470+
seconds: 15
471+
}
472+
, unknown_attributes=[x]}
473+
result: true

testing/src/main/java/dev/cel/testing/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ java_library(
8787
"//common/types:message_type_provider",
8888
"//common/types:type_providers",
8989
"//common/values:cel_byte_string",
90+
"//extensions",
9091
"//extensions:optional_library",
9192
"//runtime",
9293
"//runtime:function_binding",

testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
import dev.cel.expr.conformance.proto3.TestAllTypes;
7575
import dev.cel.expr.conformance.proto3.TestAllTypes.NestedEnum;
7676
import dev.cel.expr.conformance.proto3.TestAllTypes.NestedMessage;
77+
import dev.cel.extensions.CelExtensions;
7778
import dev.cel.extensions.CelOptionalLibrary;
7879
import dev.cel.runtime.CelAttributePattern;
7980
import dev.cel.runtime.CelEvaluationException;
@@ -153,7 +154,7 @@ protected void prepareCompiler(CelTypeProvider typeProvider) {
153154
this.celCompiler =
154155
celCompiler
155156
.toCompilerBuilder()
156-
.addLibraries(CelOptionalLibrary.INSTANCE)
157+
.addLibraries(CelOptionalLibrary.INSTANCE, CelExtensions.bindings())
157158
.setOptions(celOptions)
158159
.build();
159160
}

0 commit comments

Comments
 (0)