diff --git a/common/src/main/java/dev/cel/common/values/SelectField.java b/common/src/main/java/dev/cel/common/values/SelectField.java
index c883d315f..e94c2d817 100644
--- a/common/src/main/java/dev/cel/common/values/SelectField.java
+++ b/common/src/main/java/dev/cel/common/values/SelectField.java
@@ -82,7 +82,7 @@ public abstract class SelectField {
* Map entry decoding specification. Non-null if and only if this is a map field ({@link
* #CEL_MAP_TYPE_CODE}).
*/
- abstract @Nullable MapEntrySpec mapEntrySpec();
+ public abstract @Nullable MapEntrySpec mapEntrySpec();
// The lite runtime's FieldLiteDescriptor for this field, built from this field alone on first
// use so descriptorless hops don't rebuild it. Typed Object to keep this target free of protobuf
@@ -176,7 +176,8 @@ public static SelectField create(
}
/**
- * Creates a field selection hop for a map field with scalar values.
+ * Creates a field selection hop for a map field with scalar values, or with message values of an
+ * unspecified type.
*
* @param fieldNumber Protobuf field number.
* @param fieldName Protobuf field name.
@@ -193,15 +194,15 @@ public static SelectField createMap(
* @param fieldNumber Protobuf field number.
* @param fieldName Protobuf field name.
* @param mapEntrySpec Specification for decoding map entries.
- * @param protoTypeName Protobuf message type name of the map value. Must be non-empty if and only
- * if the map value is a message.
+ * @param protoTypeName Protobuf message type name of the map value, or empty string if
+ * unspecified. Must be empty if the map value is not a message.
*/
public static SelectField createMap(
long fieldNumber, String fieldName, MapEntrySpec mapEntrySpec, String protoTypeName) {
checkNotNull(mapEntrySpec);
checkNotNull(protoTypeName);
checkArgument(
- (mapEntrySpec.valueTypeCode() == MESSAGE_TYPE_CODE) != protoTypeName.isEmpty(),
+ protoTypeName.isEmpty() || mapEntrySpec.valueTypeCode() == MESSAGE_TYPE_CODE,
"Map value proto type name '%s' is inconsistent with value type code %s",
protoTypeName,
mapEntrySpec.valueTypeCode());
diff --git a/common/src/test/java/dev/cel/common/values/SelectFieldTest.java b/common/src/test/java/dev/cel/common/values/SelectFieldTest.java
index 7e1dfbe4f..b725300cf 100644
--- a/common/src/test/java/dev/cel/common/values/SelectFieldTest.java
+++ b/common/src/test/java/dev/cel/common/values/SelectFieldTest.java
@@ -163,8 +163,9 @@ public void create_mapTypeCode_throwsIllegalArgumentException() {
}
@Test
- public void createMap_threeArguments_success() {
- SelectField.MapEntrySpec mapEntrySpec = SelectField.MapEntrySpec.create(9, 9);
+ public void createMap_threeArguments_success(
+ @TestParameter({"9", "11"}) long valueTypeCode) { // STRING, MESSAGE of unspecified type
+ SelectField.MapEntrySpec mapEntrySpec = SelectField.MapEntrySpec.create(9, valueTypeCode);
SelectField field = SelectField.createMap(3L, "map_field", mapEntrySpec);
@@ -282,30 +283,16 @@ public void equalsAndHashCode_testedProperly() {
}
@Test
- public void createMap_protoTypeNameInconsistentWithValueTypeCode_throws(
- @TestParameter InconsistentMapValueTestCase testCase) {
- SelectField.MapEntrySpec spec = SelectField.MapEntrySpec.create(9, testCase.valueTypeCode);
+ public void createMap_scalarValueWithProtoTypeName_throws() {
+ SelectField.MapEntrySpec spec = SelectField.MapEntrySpec.create(9, 9);
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
- () -> SelectField.createMap(1L, "map_field", spec, testCase.protoTypeName));
+ () -> SelectField.createMap(1L, "map_field", spec, "proto.Name"));
assertThat(thrown)
.hasMessageThat()
- .contains("Map value proto type name '" + testCase.protoTypeName + "' is inconsistent");
- }
-
- private enum InconsistentMapValueTestCase {
- MESSAGE_VALUE_WITHOUT_TYPE_NAME(SelectField.MESSAGE_TYPE_CODE, ""),
- SCALAR_VALUE_WITH_TYPE_NAME(9, "cel.expr.conformance.proto3.TestAllTypes");
-
- private final long valueTypeCode;
- private final String protoTypeName;
-
- InconsistentMapValueTestCase(long valueTypeCode, String protoTypeName) {
- this.valueTypeCode = valueTypeCode;
- this.protoTypeName = protoTypeName;
- }
+ .contains("Map value proto type name 'proto.Name' is inconsistent with value type code 9");
}
}
diff --git a/optimizer/src/main/java/dev/cel/optimizer/optimizers/SelectOptimizer.java b/optimizer/src/main/java/dev/cel/optimizer/optimizers/SelectOptimizer.java
index 2ce52ac31..71a7aed38 100644
--- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/SelectOptimizer.java
+++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/SelectOptimizer.java
@@ -23,6 +23,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.primitives.UnsignedLong;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.protobuf.ByteString;
@@ -121,9 +122,13 @@
* // Presence tests (2-tuples)
* has(request.user.age) -> cel.@hasField(request,
* [[user_num, "user"], [age_num, "age"]])
+ *
+ * // Constant list indices and map keys between fields are inlined as bare constants
+ * request.users[0].age -> cel.@attribute(request,
+ * [[users_num, "users", 11], 0, [age_num, "age", type_code]], 0)
*
*
- * Map indexing and non-protobuf selects pass through untouched.
+ *
Dynamic or trailing indices and non-protobuf selects pass through untouched.
*
*
Limitations:
*
@@ -153,6 +158,9 @@ public final class SelectOptimizer implements CelAstOptimizer {
private static final TypeParamType TYPE_PARAM_T = TypeParamType.create("T");
+ private static final ImmutableSet> STANDARD_INDEX_OVERLOADS =
+ ImmutableSet.of(ImmutableList.of("index_list"), ImmutableList.of("index_map"));
+
/**
* Declaration for {@code cel.@attribute(operand, qualifiers, dummyOrDefVal) -> T}.
*
@@ -261,72 +269,37 @@ private void rewriteSelectChain(
boolean isHasField = topNode.expr().select().testOnly();
astToModify.source().getMacroCalls().remove(topNode.expr().id());
- List fields = new ArrayList<>();
FieldDescriptor topField =
getOptimizableField(navAst, topNode)
.orElseThrow(
() -> new IllegalStateException("Expected optimizable field on select node"));
- fields.add(topField);
+ // Walks from the leaf. Fields before an index are selected by value, so they carry their type
+ // codes even in presence tests.
+ boolean isSelectedByValue = !isHasField;
+ List qualifierLists = new ArrayList<>();
+ qualifierLists.add(newFieldQualifier(topField, isSelectedByValue, idGenerator));
CelMutableExpr currentExpr = topNode.expr().select().operand();
- while (currentExpr.getKind() == Kind.SELECT) {
- CelMutableSelect select = currentExpr.select();
- FieldDescriptor field = getOptimizableFieldForExpr(navAst, select).orElse(null);
- if (field == null) {
- break;
- }
- fields.add(field);
- currentExpr = select.operand();
- }
-
- Collections.reverse(fields);
-
- List qualifierLists = new ArrayList<>(fields.size());
- for (FieldDescriptor field : fields) {
- if (field.getType() == FieldDescriptor.Type.GROUP) {
- throw new UnsupportedOperationException(
- "Optimization of Group fields is unsupported: " + field.getFullName());
- }
- // TODO: Support Any.
- if (field.getType() == FieldDescriptor.Type.MESSAGE
- && field.getMessageType().getFullName().equals(CelTypes.ANY_MESSAGE)) {
- throw new UnsupportedOperationException(
- "Optimization of Any fields is currently unimplemented: " + field.getFullName());
- }
-
- CelMutableList qualifierElements =
- CelMutableList.create(
- CelMutableExpr.ofConstant(
- idGenerator.nextExprId(), CelConstant.ofValue((long) field.getNumber())),
- CelMutableExpr.ofConstant(
- idGenerator.nextExprId(), CelConstant.ofValue(field.getName())));
- if (!isHasField) {
- qualifierElements
- .elements()
- .add(
- CelMutableExpr.ofConstant(
- idGenerator.nextExprId(), CelConstant.ofValue(resolveTypeCode(field))));
- if (field.isMapField()) {
- Descriptor mapEntryDesc = field.getMessageType();
- FieldDescriptor keyField = mapEntryDesc.findFieldByName("key");
- FieldDescriptor valField = mapEntryDesc.findFieldByName("value");
- qualifierElements
- .elements()
- .add(
- CelMutableExpr.ofList(
- idGenerator.nextExprId(),
- CelMutableList.create(
- CelMutableExpr.ofConstant(
- idGenerator.nextExprId(),
- CelConstant.ofValue(resolveTypeCode(keyField))),
- CelMutableExpr.ofConstant(
- idGenerator.nextExprId(),
- CelConstant.ofValue(resolveTypeCode(valField))))));
+ while (true) {
+ if (currentExpr.getKind() == Kind.SELECT) {
+ FieldDescriptor field =
+ getOptimizableFieldForExpr(navAst, currentExpr.select()).orElse(null);
+ if (field != null) {
+ qualifierLists.add(newFieldQualifier(field, isSelectedByValue, idGenerator));
+ currentExpr = currentExpr.select().operand();
+ continue;
}
}
- qualifierLists.add(CelMutableExpr.ofList(idGenerator.nextExprId(), qualifierElements));
+ CelMutableExpr indexedField = getIndexedField(navAst, currentExpr).orElse(null);
+ if (indexedField == null) {
+ break;
+ }
+ qualifierLists.add(currentExpr.call().args().get(1));
+ isSelectedByValue = true;
+ currentExpr = indexedField;
}
+ Collections.reverse(qualifierLists);
CelMutableExpr qualifiersExpr =
CelMutableExpr.ofList(idGenerator.nextExprId(), CelMutableList.create(qualifierLists));
if (isHasField) {
@@ -343,6 +316,53 @@ private void rewriteSelectChain(
}
}
+ /** Returns the qualifier hop for {@code field}, with its type codes if {@code withTypeCode}. */
+ private static CelMutableExpr newFieldQualifier(
+ FieldDescriptor field, boolean withTypeCode, MonotonicIdGenerator idGenerator) {
+ if (field.getType() == FieldDescriptor.Type.GROUP) {
+ throw new UnsupportedOperationException(
+ "Optimization of Group fields is unsupported: " + field.getFullName());
+ }
+ // TODO: Support Any.
+ if (field.getType() == FieldDescriptor.Type.MESSAGE
+ && field.getMessageType().getFullName().equals(CelTypes.ANY_MESSAGE)) {
+ throw new UnsupportedOperationException(
+ "Optimization of Any fields is currently unimplemented: " + field.getFullName());
+ }
+
+ CelMutableList qualifierElements =
+ CelMutableList.create(
+ CelMutableExpr.ofConstant(
+ idGenerator.nextExprId(), CelConstant.ofValue((long) field.getNumber())),
+ CelMutableExpr.ofConstant(
+ idGenerator.nextExprId(), CelConstant.ofValue(field.getName())));
+ if (withTypeCode) {
+ qualifierElements
+ .elements()
+ .add(
+ CelMutableExpr.ofConstant(
+ idGenerator.nextExprId(), CelConstant.ofValue(resolveTypeCode(field))));
+ if (field.isMapField()) {
+ Descriptor mapEntryDesc = field.getMessageType();
+ FieldDescriptor keyField = mapEntryDesc.findFieldByName("key");
+ FieldDescriptor valField = mapEntryDesc.findFieldByName("value");
+ qualifierElements
+ .elements()
+ .add(
+ CelMutableExpr.ofList(
+ idGenerator.nextExprId(),
+ CelMutableList.create(
+ CelMutableExpr.ofConstant(
+ idGenerator.nextExprId(),
+ CelConstant.ofValue(resolveTypeCode(keyField))),
+ CelMutableExpr.ofConstant(
+ idGenerator.nextExprId(),
+ CelConstant.ofValue(resolveTypeCode(valField))))));
+ }
+ }
+ return CelMutableExpr.ofList(idGenerator.nextExprId(), qualifierElements);
+ }
+
private static long resolveTypeCode(FieldDescriptor field) {
if (field.isMapField()) {
return CEL_MAP_TYPE_CODE;
@@ -351,8 +371,39 @@ private static long resolveTypeCode(FieldDescriptor field) {
}
private boolean isTopOfSelectChain(CelNavigableMutableAst navAst, CelNavigableMutableExpr node) {
- return getOptimizableField(navAst, node).isPresent()
- && !node.parent().flatMap(parent -> getOptimizableField(navAst, parent)).isPresent();
+ if (!getOptimizableField(navAst, node).isPresent()) {
+ return false;
+ }
+ Optional parent = node.parent();
+ // A constant index into this field continues the chain into a field of the element.
+ if (parent.isPresent() && getIndexedField(navAst, parent.get().expr()).isPresent()) {
+ parent = parent.get().parent();
+ }
+ return !parent.flatMap(p -> getOptimizableField(navAst, p)).isPresent();
+ }
+
+ /**
+ * Returns the protobuf field select that {@code expr} indexes by a constant through a standard
+ * {@code _[_]} overload, if the field is optimizable.
+ */
+ private Optional getIndexedField(
+ CelNavigableMutableAst navAst, CelMutableExpr expr) {
+ // `field.key` on a map isn't inlined: its missing-key error differs from `field['key']`'s.
+ boolean isStandardIndex =
+ navAst
+ .getAst()
+ .getReference(expr.id())
+ .map(reference -> STANDARD_INDEX_OVERLOADS.contains(reference.overloadIds()))
+ .orElse(false);
+ if (!isStandardIndex || expr.call().args().get(1).getKind() != Kind.CONSTANT) {
+ return Optional.empty();
+ }
+ CelMutableExpr fieldExpr = expr.call().args().get(0);
+ if (fieldExpr.getKind() != Kind.SELECT
+ || !getOptimizableFieldForExpr(navAst, fieldExpr.select()).isPresent()) {
+ return Optional.empty();
+ }
+ return Optional.of(fieldExpr);
}
private Optional getOptimizableField(
diff --git a/optimizer/src/test/java/dev/cel/optimizer/optimizers/SelectOptimizerTest.java b/optimizer/src/test/java/dev/cel/optimizer/optimizers/SelectOptimizerTest.java
index 4f85a426e..ce5472ebf 100644
--- a/optimizer/src/test/java/dev/cel/optimizer/optimizers/SelectOptimizerTest.java
+++ b/optimizer/src/test/java/dev/cel/optimizer/optimizers/SelectOptimizerTest.java
@@ -66,8 +66,12 @@
import dev.cel.parser.CelStandardMacro;
import dev.cel.parser.CelUnparser;
import dev.cel.parser.CelUnparserFactory;
+import dev.cel.runtime.CelAttribute;
+import dev.cel.runtime.CelAttributePattern;
import dev.cel.runtime.CelEvaluationException;
import dev.cel.runtime.CelRuntime.Program;
+import dev.cel.runtime.CelUnknownSet;
+import dev.cel.runtime.PartialVars;
import dev.cel.testing.CelRuntimeFlavor;
import java.time.Duration;
import java.time.Instant;
@@ -337,10 +341,49 @@ private enum RewriteTestCase {
+ " {\"\": google.protobuf.Duration{}})"),
MAP_FIELD_INDEXING(
"msg.map_int64_message[1].bb",
- "cel.@attribute("
- + "cel.@attribute(msg, [[95, \"map_int64_message\", -1, [3, 11]]],"
- + " {0: cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}})[1], "
+ "cel.@attribute(msg, [[95, \"map_int64_message\", -1, [3, 11]], 1, [1, \"bb\", 5]], 0)"),
+ REPEATED_FIELD_INDEXING(
+ "msg.repeated_nested_message[0].bb",
+ "cel.@attribute(msg, [[51, \"repeated_nested_message\", 11], 0, [1, \"bb\", 5]], 0)"),
+ MAP_AND_REPEATED_FIELD_INDEXING(
+ "msg.map_int64_nested_type[1].child.payload.repeated_nested_message[0].bb",
+ "cel.@attribute(msg, [[62, \"map_int64_nested_type\", -1, [3, 11]], 1, "
+ + "[1, \"child\", 11], [2, \"payload\", 11], "
+ + "[51, \"repeated_nested_message\", 11], 0, [1, \"bb\", 5]], 0)"),
+ MAP_FIELD_UINT_KEY_INDEXING(
+ "msg.map_uint64_message[1u].bb",
+ "cel.@attribute(msg, [[217, \"map_uint64_message\", -1, [4, 11]], 1u, "
+ + "[1, \"bb\", 5]], 0)"),
+ MAP_FIELD_BOOL_KEY_INDEXING(
+ "msg.map_bool_message[true].bb",
+ "cel.@attribute(msg, [[73, \"map_bool_message\", -1, [8, 11]], true, "
+ + "[1, \"bb\", 5]], 0)"),
+ REPEATED_FIELD_HAS_THROUGH_INDEX(
+ "has(msg.repeated_nested_message[0].bb)",
+ "cel.@hasField(msg, [[51, \"repeated_nested_message\", 11], 0, [1, \"bb\"]])"),
+ MAP_FIELD_HAS_THROUGH_INDEX(
+ "has(msg.map_int64_nested_type[1].child.payload.single_int64)",
+ "cel.@hasField(msg, [[62, \"map_int64_nested_type\", -1, [3, 11]], 1, "
+ + "[1, \"child\"], [2, \"payload\"], [2, \"single_int64\"]])"),
+ HAS_THROUGH_TWO_INDICES(
+ "has(msg.map_int64_nested_type[1].child.payload.repeated_nested_message[0].bb)",
+ "cel.@hasField(msg, [[62, \"map_int64_nested_type\", -1, [3, 11]], 1, "
+ + "[1, \"child\", 11], [2, \"payload\", 11], "
+ + "[51, \"repeated_nested_message\", 11], 0, [1, \"bb\"]])"),
+ REPEATED_FIELD_DYNAMIC_INDEX_STOPS_AT_INDEX(
+ "msg.repeated_nested_message[x].bb",
+ "cel.@attribute(cel.@attribute(msg, [[51, \"repeated_nested_message\", 11]],"
+ + " [cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}])[x], "
+ "[[1, \"bb\", 5]], 0)"),
+ REPEATED_FIELD_TRAILING_INDEX_STOPS_AT_INDEX(
+ "msg.repeated_nested_message[0]",
+ "cel.@attribute(msg, [[51, \"repeated_nested_message\", 11]],"
+ + " [cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}])[0]"),
+ REPEATED_FIELD_INDEX_HOLDING_SELECT(
+ "msg.repeated_nested_message[msg.single_int64].bb",
+ "cel.@attribute(cel.@attribute(msg, [[51, \"repeated_nested_message\", 11]],"
+ + " [cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}])"
+ + "[cel.@attribute(msg, [[2, \"single_int64\", 3]], 0)], [[1, \"bb\", 5]], 0)"),
MAP_FIELD_SELECT_CHAIN_STOPS_AT_MAP_BOUNDARY(
"map_var_msg.key.single_nested_message.bb",
"cel.@attribute(map_var_msg.key, "
@@ -355,6 +398,9 @@ private enum RewriteTestCase {
MAP_FIELD_HAS_CHAIN_STOPS_AT_MAP_BOUNDARY(
"has(map_var_msg.key.single_nested_message.bb)",
"cel.@hasField(map_var_msg.key, [[21, \"single_nested_message\"], [1, \"bb\"]])"),
+ MAP_VALUE_INDEX_STOPS_AT_MAP_BOUNDARY(
+ "{'a': [msg]}.a[0].single_int64",
+ "cel.@attribute({\"a\": [msg]}.a[0], [[2, \"single_int64\", 3]], 0)"),
PROTO_MAP_FIELD_SELECT_STOPS_AT_MAP_BOUNDARY(
"msg.map_string_message.key.bb",
"cel.@attribute("
@@ -907,6 +953,66 @@ private enum NativeSelectEvaluationTestCase {
ImmutableMap.of(
"proto2_msg", dev.cel.expr.conformance.proto2.TestAllTypes.getDefaultInstance()),
false),
+ REPEATED_FIELD_INDEX(
+ "msg.repeated_nested_message[1].bb",
+ ImmutableMap.of("msg", newTestAllTypesWithRepeatedNestedMessages(10, 20)),
+ 20L),
+ MAP_FIELD_INDEX(
+ "msg.map_int64_message[2].bb",
+ ImmutableMap.of(
+ "msg",
+ TestAllTypes.newBuilder()
+ .putMapInt64Message(2, TestAllTypes.NestedMessage.newBuilder().setBb(42).build())
+ .build()),
+ 42L),
+ MAP_FIELD_UINT_KEY_INDEX(
+ "msg.map_uint64_message[2u].bb",
+ ImmutableMap.of(
+ "msg",
+ TestAllTypes.newBuilder()
+ .putMapUint64Message(2, TestAllTypes.NestedMessage.newBuilder().setBb(42).build())
+ .build()),
+ 42L),
+ MAP_AND_REPEATED_FIELD_INDEX(
+ "msg.map_int64_nested_type[1].child.payload.repeated_nested_message[1].bb",
+ ImmutableMap.of(
+ "msg",
+ TestAllTypes.newBuilder()
+ .putMapInt64NestedType(
+ 1,
+ dev.cel.expr.conformance.proto3.NestedTestAllTypes.newBuilder()
+ .setChild(
+ dev.cel.expr.conformance.proto3.NestedTestAllTypes.newBuilder()
+ .setPayload(newTestAllTypesWithRepeatedNestedMessages(10, 20)))
+ .build())
+ .build()),
+ 20L),
+ INDEXED_ELEMENT_UNSET_FIELD(
+ "msg.repeated_nested_message[0].bb",
+ ImmutableMap.of("msg", newTestAllTypesWithRepeatedNestedMessages(0)),
+ 0L),
+ HAS_FIELD_THROUGH_INDEX_PRESENT(
+ "has(msg.repeated_nested_message[1].bb)",
+ ImmutableMap.of("msg", newTestAllTypesWithRepeatedNestedMessages(0, 20)),
+ true),
+ HAS_FIELD_THROUGH_INDEX_ABSENT(
+ "has(msg.repeated_nested_message[0].bb)",
+ ImmutableMap.of("msg", newTestAllTypesWithRepeatedNestedMessages(0, 20)),
+ false),
+ HAS_FIELD_THROUGH_TWO_INDICES(
+ "has(msg.map_int64_nested_type[1].child.payload.repeated_nested_message[1].bb)",
+ ImmutableMap.of(
+ "msg",
+ TestAllTypes.newBuilder()
+ .putMapInt64NestedType(
+ 1,
+ dev.cel.expr.conformance.proto3.NestedTestAllTypes.newBuilder()
+ .setChild(
+ dev.cel.expr.conformance.proto3.NestedTestAllTypes.newBuilder()
+ .setPayload(newTestAllTypesWithRepeatedNestedMessages(10, 20)))
+ .build())
+ .build()),
+ true),
SELECT_ON_MAP_VALUE_POPULATED(
"map_var_msg.key.single_nested_message.bb",
ImmutableMap.of(
@@ -1066,6 +1172,104 @@ public void optimizeAndEvaluate_nativeSelectAndHasField_matchesUnoptimized(
assertThat(optimizedResult).isEqualTo(unoptimizedResult);
}
+ @Test
+ public void optimize_constantIndexThroughCustomOverload_leavesIndexNested() throws Exception {
+ StructTypeReference nestedMessageType =
+ StructTypeReference.create(TestAllTypes.NestedMessage.getDescriptor().getFullName());
+ Cel celWithCustomIndex =
+ cel.toCelBuilder()
+ .addFunctionDeclarations(
+ CelFunctionDecl.newFunctionDeclaration(
+ "_[_]",
+ CelOverloadDecl.newGlobalOverload(
+ "custom_index_list_string",
+ nestedMessageType,
+ ListType.create(nestedMessageType),
+ SimpleType.STRING)))
+ .build();
+ CelAbstractSyntaxTree ast =
+ celWithCustomIndex.compile("msg.repeated_nested_message['0'].bb").getAst();
+
+ CelAbstractSyntaxTree optimizedAst = newSelectOptimizer(celWithCustomIndex).optimize(ast);
+
+ assertThat(CEL_UNPARSER.unparse(optimizedAst))
+ .isEqualTo(
+ "cel.@attribute(cel.@attribute(msg, [[51, \"repeated_nested_message\", 11]],"
+ + " [cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}])[\"0\"],"
+ + " [[1, \"bb\", 5]], 0)");
+ }
+
+ @Test
+ public void optimizeAndEvaluate_missingIndexedElement_throwsSameErrorAsUnoptimized(
+ @TestParameter({
+ "msg.repeated_nested_message[1].bb",
+ "msg.repeated_nested_message[5000000000].bb",
+ "msg.map_int64_message[2].bb",
+ "has(msg.repeated_nested_message[1].bb)",
+ "has(msg.map_int64_message[2].bb)"
+ })
+ String expression)
+ throws Exception {
+ assumeTrue(runtimeFlavor == CelRuntimeFlavor.PLANNER);
+ CelAbstractSyntaxTree ast = cel.compile(expression).getAst();
+ Program unoptimizedProgram = cel.createProgram(ast);
+ Program optimizedProgram = cel.createProgram(celOptimizer.optimize(ast));
+ ImmutableMap input =
+ ImmutableMap.of("msg", newTestAllTypesWithRepeatedNestedMessages(10));
+
+ CelEvaluationException unoptimizedError =
+ assertThrows(CelEvaluationException.class, () -> unoptimizedProgram.eval(input));
+ CelEvaluationException optimizedError =
+ assertThrows(CelEvaluationException.class, () -> optimizedProgram.eval(input));
+
+ assertThat(optimizedError.getErrorCode()).isEqualTo(unoptimizedError.getErrorCode());
+ assertThat(optimizedError)
+ .hasCauseThat()
+ .hasMessageThat()
+ .isEqualTo(unoptimizedError.getCause().getMessage());
+ }
+
+ @Test
+ public void optimizeAndEvaluate_partialVarsOnIndexedElement_returnsSameUnknownAsUnoptimized()
+ throws Exception {
+ assumeTrue(runtimeFlavor == CelRuntimeFlavor.PLANNER);
+ CelAbstractSyntaxTree ast = cel.compile("msg.repeated_nested_message[0].bb").getAst();
+ CelAbstractSyntaxTree optimizedAst = celOptimizer.optimize(ast);
+ PartialVars partialVars = newRepeatedNestedMessagePartialVars(/* unknownIndex= */ 0);
+
+ CelUnknownSet unoptimizedResult = (CelUnknownSet) cel.createProgram(ast).eval(partialVars);
+ CelUnknownSet optimizedResult =
+ (CelUnknownSet) cel.createProgram(optimizedAst).eval(partialVars);
+
+ assertThat(optimizedResult.attributes()).isEqualTo(unoptimizedResult.attributes());
+ assertThat(optimizedResult.attributes())
+ .containsExactly(
+ CelAttribute.fromQualifiedIdentifier("msg.repeated_nested_message")
+ .qualify(CelAttribute.Qualifier.ofInt(0)));
+ }
+
+ @Test
+ public void optimizeAndEvaluate_partialVarsOnOtherElement_returnsSameValueAsUnoptimized()
+ throws Exception {
+ assumeTrue(runtimeFlavor == CelRuntimeFlavor.PLANNER);
+ CelAbstractSyntaxTree ast = cel.compile("msg.repeated_nested_message[0].bb").getAst();
+ CelAbstractSyntaxTree optimizedAst = celOptimizer.optimize(ast);
+ PartialVars partialVars = newRepeatedNestedMessagePartialVars(/* unknownIndex= */ 1);
+
+ Object unoptimizedResult = cel.createProgram(ast).eval(partialVars);
+ Object optimizedResult = cel.createProgram(optimizedAst).eval(partialVars);
+
+ assertThat(optimizedResult).isEqualTo(unoptimizedResult);
+ assertThat(optimizedResult).isEqualTo(10L);
+ }
+
+ private static PartialVars newRepeatedNestedMessagePartialVars(long unknownIndex) {
+ return PartialVars.of(
+ ImmutableMap.of("msg", newTestAllTypesWithRepeatedNestedMessages(10, 20)),
+ CelAttributePattern.fromQualifiedIdentifier("msg.repeated_nested_message")
+ .qualify(CelAttribute.Qualifier.ofInt(unknownIndex)));
+ }
+
@Test
public void optimize_comprehension_rewritesSelectInLoopStep() throws Exception {
CelAbstractSyntaxTree ast = cel.compile("[msg].map(m, m.single_int64)").getAst();
@@ -1517,6 +1721,14 @@ private static CelOptimizer newSelectOptimizer(Cel cel) {
.build();
}
+ private static TestAllTypes newTestAllTypesWithRepeatedNestedMessages(int... bbs) {
+ TestAllTypes.Builder builder = TestAllTypes.newBuilder();
+ for (int bb : bbs) {
+ builder.addRepeatedNestedMessage(TestAllTypes.NestedMessage.newBuilder().setBb(bb));
+ }
+ return builder.build();
+ }
+
private static NestedTestAllTypes newNestedTestAllTypes(long singleInt64) {
// Proto2 TestAllTypes requires FQN due to simple-name collision with proto3 TestAllTypes.
return NestedTestAllTypes.newBuilder()
diff --git a/runtime/src/main/java/dev/cel/runtime/CelAttributePattern.java b/runtime/src/main/java/dev/cel/runtime/CelAttributePattern.java
index ff5f3f5bf..9b9f70112 100644
--- a/runtime/src/main/java/dev/cel/runtime/CelAttributePattern.java
+++ b/runtime/src/main/java/dev/cel/runtime/CelAttributePattern.java
@@ -19,6 +19,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.primitives.UnsignedLong;
import com.google.errorprone.annotations.Immutable;
/**
@@ -124,9 +125,7 @@ public boolean isMatch(CelAttribute attribute) {
int qualifierCount = min(qualifiers().size(), attribute.qualifiers().size());
int i = 0;
for (; i < qualifierCount; i++) {
- CelAttribute.Qualifier pattern = qualifiers().get(i);
- CelAttribute.Qualifier qualifier = attribute.qualifiers().get(i);
- if (pattern.kind() != CelAttribute.Qualifier.Kind.WILD_CARD && !pattern.equals(qualifier)) {
+ if (!matches(qualifiers().get(i), attribute.qualifiers().get(i))) {
return false;
}
}
@@ -154,9 +153,7 @@ public boolean isPartialMatch(CelAttribute attribute) {
}
int qualifierCount = min(qualifiers().size(), attribute.qualifiers().size());
for (int i = 0; i < qualifierCount; i++) {
- CelAttribute.Qualifier pattern = qualifiers().get(i);
- CelAttribute.Qualifier qualifier = attribute.qualifiers().get(i);
- if (pattern.kind() != CelAttribute.Qualifier.Kind.WILD_CARD && !pattern.equals(qualifier)) {
+ if (!matches(qualifiers().get(i), attribute.qualifiers().get(i))) {
return false;
}
}
@@ -165,6 +162,24 @@ public boolean isPartialMatch(CelAttribute attribute) {
return true;
}
+ // Numeric qualifiers match by value across int and uint, as map keys and list indices do.
+ private static boolean matches(CelAttribute.Qualifier pattern, CelAttribute.Qualifier qualifier) {
+ if (pattern.kind() == CelAttribute.Qualifier.Kind.WILD_CARD || pattern.equals(qualifier)) {
+ return true;
+ }
+ if (pattern.kind() == CelAttribute.Qualifier.Kind.AS_INT
+ && qualifier.kind() == CelAttribute.Qualifier.Kind.AS_UINT) {
+ return intEqualsUint(pattern.asInt(), qualifier.asUint());
+ }
+ return pattern.kind() == CelAttribute.Qualifier.Kind.AS_UINT
+ && qualifier.kind() == CelAttribute.Qualifier.Kind.AS_INT
+ && intEqualsUint(qualifier.asInt(), pattern.asUint());
+ }
+
+ private static boolean intEqualsUint(long intValue, UnsignedLong uintValue) {
+ return intValue >= 0 && UnsignedLong.valueOf(intValue).equals(uintValue);
+ }
+
/**
* Return an appropriate attribute for a pattern match.
*
diff --git a/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java b/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java
index 6dbba80cd..7444ffef1 100644
--- a/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java
+++ b/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java
@@ -133,11 +133,11 @@ public static A indexList(List list, Number index) {
.map(v -> indexList(list, v))
.orElseThrow(() -> new CelIndexOutOfBoundsException(index.doubleValue()));
}
- int castIndex = Ints.checkedCast(index.longValue());
- if (castIndex < 0 || castIndex >= list.size()) {
- throw new CelIndexOutOfBoundsException(castIndex);
+ long longIndex = index.longValue();
+ if (longIndex < 0 || longIndex >= list.size()) {
+ throw new CelIndexOutOfBoundsException(index);
}
- return list.get(castIndex);
+ return list.get((int) longIndex);
}
// Integer Arithmetic
diff --git a/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel b/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel
index c25bd9ca0..7258ffe34 100644
--- a/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel
+++ b/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel
@@ -320,6 +320,9 @@ java_library(
name = "eval_index",
srcs = ["EvalIndex.java"],
deps = [
+ ":attribute",
+ ":eval_attribute",
+ ":eval_const",
":eval_helpers",
":planned_interpretable",
"//common/ast",
@@ -329,6 +332,7 @@ java_library(
"//runtime:interpretable",
"//runtime:resolved_overload",
"@maven//:com_google_errorprone_error_prone_annotations",
+ "@maven//:com_google_guava_guava",
],
)
@@ -552,8 +556,11 @@ java_library(
"//common/values:cel_byte_string",
"//common/values:optimized_select_traversal",
"//common/values:select_field",
+ "//runtime:runtime_equality",
+ "//runtime:runtime_helpers",
"@maven//:com_google_errorprone_error_prone_annotations",
"@maven//:com_google_guava_guava",
+ "@maven//:org_jspecify_jspecify",
],
)
@@ -985,6 +992,9 @@ cel_android_library(
name = "eval_index_android",
srcs = ["EvalIndex.java"],
deps = [
+ ":attribute_android",
+ ":eval_attribute_android",
+ ":eval_const_android",
":eval_helpers_android",
":planned_interpretable_android",
"//common/ast:ast_android",
@@ -994,6 +1004,7 @@ cel_android_library(
"//runtime:interpretable_android",
"//runtime:resolved_overload_android",
"@maven//:com_google_errorprone_error_prone_annotations",
+ "@maven_android//:com_google_guava_guava",
],
)
@@ -1217,7 +1228,10 @@ cel_android_library(
"//common/values:optimized_select_traversal_android",
"//common/values:select_field_android",
"//common/values:values_android",
+ "//runtime:runtime_equality_android",
+ "//runtime:runtime_helpers_android",
"@maven//:com_google_errorprone_error_prone_annotations",
+ "@maven//:org_jspecify_jspecify",
"@maven_android//:com_google_guava_guava",
],
)
diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java
index 012432f45..bca40c235 100644
--- a/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java
+++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java
@@ -59,16 +59,8 @@ static Object evalStrictly(
Object val;
try {
val = interpretable.eval(resolver, frame);
- } catch (LocalizedEvaluationException e) {
- // Already localized - propagate as-is to preserve inner expression ID
- throw e;
- } catch (CelRuntimeException e) {
- // Wrap with current interpretable's location
- throw new LocalizedEvaluationException(e, interpretable.expr().id());
} catch (Exception e) {
- // Wrap generic exceptions with location
- throw new LocalizedEvaluationException(
- e, CelErrorCode.INTERNAL_ERROR, interpretable.expr().id());
+ throw localize(e, interpretable.expr().id());
}
if (val instanceof ErrorValue) {
throw ((ErrorValue) val).value();
@@ -76,6 +68,17 @@ static Object evalStrictly(
return val;
}
+ /** Localizes {@code e} to {@code exprId} unless an inner expression already did. */
+ static LocalizedEvaluationException localize(Exception e, long exprId) {
+ if (e instanceof LocalizedEvaluationException) {
+ return (LocalizedEvaluationException) e;
+ }
+ if (e instanceof CelRuntimeException) {
+ return new LocalizedEvaluationException((CelRuntimeException) e, exprId);
+ }
+ return new LocalizedEvaluationException(e, CelErrorCode.INTERNAL_ERROR, exprId);
+ }
+
static Object dispatch(
String functionName,
CelResolvedOverload overload,
diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalIndex.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalIndex.java
index d7fec0088..d012cea03 100644
--- a/runtime/src/main/java/dev/cel/runtime/planner/EvalIndex.java
+++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalIndex.java
@@ -17,6 +17,7 @@
import static dev.cel.runtime.planner.EvalHelpers.evalNonstrictly;
import static dev.cel.runtime.planner.EvalHelpers.evalStrictly;
+import com.google.common.primitives.UnsignedLong;
import com.google.errorprone.annotations.Immutable;
import dev.cel.common.ast.CelExpr;
import dev.cel.common.values.CelValueConverter;
@@ -53,16 +54,79 @@ Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) throws CelEva
functionName, resolvedOverload, celValueConverter, targetVal, indexVal);
}
- static EvalIndex create(
+ /** Plans {@code target[index]}. */
+ static PlannedInterpretable create(
CelExpr expr,
String functionName,
CelResolvedOverload resolvedOverload,
PlannedInterpretable target,
PlannedInterpretable index,
CelValueConverter celValueConverter) {
+ // Qualifies the attribute so unknown patterns match through constant indices, as in CEL-C++.
+ if (target instanceof EvalAttribute
+ && index instanceof EvalConstant
+ && resolvedOverload.isStrict()) {
+ Object constant = ((EvalConstant) index).constantValue();
+ // Kinds representable as a CelAttribute.Qualifier.
+ if (constant instanceof Long
+ || constant instanceof UnsignedLong
+ || constant instanceof String
+ || constant instanceof Boolean) {
+ return ((EvalAttribute) target)
+ .addQualifier(
+ expr,
+ new ConstantIndexQualifier(
+ expr.id(), resolvedOverload, constant, celValueConverter));
+ }
+ }
return new EvalIndex(expr, functionName, resolvedOverload, target, index, celValueConverter);
}
+ /** Indexes an attribute's value by a constant through the {@code _[_]} overload. */
+ @Immutable
+ private static final class ConstantIndexQualifier implements Qualifier {
+ private final long exprId;
+ private final CelResolvedOverload resolvedOverload;
+
+ @SuppressWarnings("Immutable") // Immutable CEL constant.
+ private final Object index;
+
+ private final CelValueConverter celValueConverter;
+
+ @Override
+ public Object value() {
+ return index;
+ }
+
+ @Override
+ public Object qualify(Object operand) {
+ // Mirrors EvalIndex: the overload receives the same adapted target, and failures are
+ // localized to the index expression.
+ try {
+ return celValueConverter.toTraversalTarget(
+ EvalHelpers.dispatch(
+ resolvedOverload.getFunctionName(),
+ resolvedOverload,
+ celValueConverter,
+ EvalHelpers.convertAndAdaptResult(celValueConverter, operand),
+ index));
+ } catch (Exception e) {
+ throw EvalHelpers.localize(e, exprId);
+ }
+ }
+
+ private ConstantIndexQualifier(
+ long exprId,
+ CelResolvedOverload resolvedOverload,
+ Object index,
+ CelValueConverter celValueConverter) {
+ this.exprId = exprId;
+ this.resolvedOverload = resolvedOverload;
+ this.index = index;
+ this.celValueConverter = celValueConverter;
+ }
+ }
+
private EvalIndex(
CelExpr expr,
String functionName,
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 d83e74f00..16e036559 100644
--- a/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java
+++ b/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java
@@ -114,9 +114,9 @@ public Object resolve(long exprId, GlobalResolver ctx, ExecutionFrame frame) {
}
// The name itself could be a fully qualified reference to an enum value
- // (e.g: my.enum_type.BAR)
+ // (e.g: my.enum_type.BAR), which can't be qualified further.
int lastDotIndex = name.lastIndexOf('.');
- if (lastDotIndex > 0) {
+ if (lastDotIndex > 0 && qualifiers.isEmpty()) {
String enumTypeName = name.substring(0, lastDotIndex);
String enumValueQualifier = name.substring(lastDotIndex + 1);
diff --git a/runtime/src/main/java/dev/cel/runtime/planner/OptimizedSelectPlanner.java b/runtime/src/main/java/dev/cel/runtime/planner/OptimizedSelectPlanner.java
index 6846e8d74..033ab1f48 100644
--- a/runtime/src/main/java/dev/cel/runtime/planner/OptimizedSelectPlanner.java
+++ b/runtime/src/main/java/dev/cel/runtime/planner/OptimizedSelectPlanner.java
@@ -31,9 +31,14 @@
import dev.cel.common.values.OptimizedSelectTraversal;
import dev.cel.common.values.OptionalValue;
import dev.cel.common.values.SelectField;
+import dev.cel.runtime.RuntimeEquality;
+import dev.cel.runtime.RuntimeHelpers;
import java.time.Duration;
import java.time.Instant;
+import java.util.List;
+import java.util.Map;
import java.util.Optional;
+import org.jspecify.annotations.Nullable;
/**
* Plans optimizer-rewritten select chains ({@code cel.@attribute}) and presence tests ({@code
@@ -46,56 +51,87 @@ final class OptimizedSelectPlanner {
static final String CEL_ATTRIBUTE_FUNCTION_NAME = "cel.@attribute";
static final String CEL_HAS_FIELD_FUNCTION_NAME = "cel.@hasField";
+ // A list index is an int, as typed by an INT64 (3) map key.
+ private static final long LIST_INDEX_TYPE_CODE = 3;
+
private final AttributeFactory attributeFactory;
private final CelValueConverter celValueConverter;
+ private final RuntimeEquality runtimeEquality;
static OptimizedSelectPlanner create(
- AttributeFactory attributeFactory, CelValueConverter celValueConverter) {
- return new OptimizedSelectPlanner(attributeFactory, celValueConverter);
+ AttributeFactory attributeFactory,
+ CelValueConverter celValueConverter,
+ RuntimeEquality runtimeEquality) {
+ return new OptimizedSelectPlanner(attributeFactory, celValueConverter, runtimeEquality);
}
PlannedInterpretable plan(CelExpr expr, String functionName, OperandPlanner operandPlanner) {
ImmutableList args = expr.call().args();
- ImmutableList selectFields;
- boolean isPresenceTest;
- switch (functionName) {
- case CEL_ATTRIBUTE_FUNCTION_NAME:
- {
- checkArgument(
- args.size() == 3, "Expected 3 arguments for %s, found %s", functionName, args.size());
- selectFields = unpackAttributeFields(args.get(1), args.get(2));
- isPresenceTest = false;
- break;
- }
- case CEL_HAS_FIELD_FUNCTION_NAME:
- {
- checkArgument(
- args.size() == 2, "Expected 2 arguments for %s, found %s", functionName, args.size());
- selectFields = unpackHasFieldFields(args.get(1));
- isPresenceTest = true;
- break;
- }
- default:
- throw new IllegalArgumentException(
- "Unsupported optimized select function: " + functionName);
+ boolean isPresenceTest = functionName.equals(CEL_HAS_FIELD_FUNCTION_NAME);
+ int expectedArgCount = isPresenceTest ? 2 : 3;
+ checkArgument(
+ args.size() == expectedArgCount,
+ "Expected %s arguments for %s, found %s",
+ expectedArgCount,
+ functionName,
+ args.size());
+
+ // Constant index hops split the qualifiers into field segments, each but the last ending at the
+ // repeated or map field that the following index selects from.
+ ImmutableList hops = unpackQualifierHops(args.get(1));
+ ImmutableList.Builder qualifiers = ImmutableList.builder();
+ int segmentStart = 0;
+ for (int i = 0; i < hops.size(); i++) {
+ CelExpr hop = hops.get(i);
+ if (hop.getKind() != Kind.CONSTANT) {
+ continue;
+ }
+ checkArgument(
+ i > segmentStart && i < hops.size() - 1,
+ "Index qualifier hop must be between field hops: %s",
+ hop);
+ ImmutableList fields =
+ unpackAttributeFields(hops.subList(segmentStart, i), /* dummyOrDefExpr= */ null);
+ addSegmentQualifiers(qualifiers, fields, /* isPresenceTest= */ false);
+ SelectField.MapEntrySpec mapEntrySpec = Iterables.getLast(fields).mapEntrySpec();
+ long keyTypeCode = mapEntrySpec != null ? mapEntrySpec.keyTypeCode() : LIST_INDEX_TYPE_CODE;
+ qualifiers.add(
+ new IndexQualifier(
+ resolveScalarDefaultValue(keyTypeCode, hop), runtimeEquality, celValueConverter));
+ segmentStart = i + 1;
}
+ ImmutableList leafSegment = hops.subList(segmentStart, hops.size());
+ addSegmentQualifiers(
+ qualifiers,
+ isPresenceTest
+ ? unpackHasFieldFields(leafSegment)
+ : unpackAttributeFields(leafSegment, args.get(2)),
+ isPresenceTest);
PlannedInterpretable operand = operandPlanner.plan(args.get(0));
Attribute qualified = PlannerHelpers.resolveBaseAttribute(operand, attributeFactory);
-
- int lastIndex = selectFields.size() - 1;
- for (int i = 0; i < lastIndex; i++) {
- qualified = qualified.addQualifier(new PassthroughQualifier(selectFields.get(i).fieldName()));
+ for (Qualifier qualifier : qualifiers.build()) {
+ qualified = qualified.addQualifier(qualifier);
}
- qualified =
- qualified.addQualifier(
- new OptimizedSelectQualifier(selectFields, celValueConverter, isPresenceTest));
return EvalAttribute.create(expr, qualified);
}
+ private void addSegmentQualifiers(
+ ImmutableList.Builder qualifiers,
+ ImmutableList fields,
+ boolean isPresenceTest) {
+ for (int i = 0; i < fields.size() - 1; i++) {
+ qualifiers.add(new PassthroughQualifier(fields.get(i).fieldName()));
+ }
+ qualifiers.add(new OptimizedSelectQualifier(fields, celValueConverter, isPresenceTest));
+ }
+
+ /**
+ * Unpacks the field hops of a {@code cel.@attribute} segment. The last hop is the leaf typed by
+ * {@code dummyOrDefExpr}, or, if null, the repeated or map field indexed by the next hop.
+ */
private static ImmutableList unpackAttributeFields(
- CelExpr qualifiersExpr, CelExpr dummyOrDefExpr) {
- ImmutableList elements = unpackQualifierHops(qualifiersExpr);
+ ImmutableList elements, @Nullable CelExpr dummyOrDefExpr) {
ImmutableList.Builder fieldsBuilder =
ImmutableList.builderWithExpectedSize(elements.size());
for (int i = 0; i < elements.size(); i++) {
@@ -124,6 +160,8 @@ private static ImmutableList unpackAttributeFields(
"Non-leaf qualifier hop must have MESSAGE type code (11), found: %s",
rawTypeCode);
fieldsBuilder.add(SelectField.create(fieldNumber, fieldName, rawTypeCode));
+ } else if (dummyOrDefExpr == null) {
+ fieldsBuilder.add(parseIndexedField(fieldNumber, fieldName, rawTypeCode, hopElements));
} else if (rawTypeCode == SelectField.CEL_MAP_TYPE_CODE) {
checkArgument(
hopElements.size() == 4,
@@ -163,6 +201,25 @@ private static SelectField parseMapLeaf(
return SelectField.createMap(fieldNumber, fieldName, mapEntrySpec);
}
+ /** Parses a repeated message or a message-valued map field hop that an index hop follows. */
+ private static SelectField parseIndexedField(
+ long fieldNumber, String fieldName, long typeCode, ImmutableList hopElements) {
+ if (typeCode == SelectField.CEL_MAP_TYPE_CODE && hopElements.size() == 4) {
+ SelectField.MapEntrySpec mapEntrySpec = parseMapEntrySpec(hopElements.get(3));
+ checkArgument(
+ mapEntrySpec.valueTypeCode() == SelectField.MESSAGE_TYPE_CODE,
+ "Indexed map qualifier hop must have MESSAGE values (11), found: %s",
+ mapEntrySpec.valueTypeCode());
+ return SelectField.createMap(fieldNumber, fieldName, mapEntrySpec);
+ }
+ checkArgument(
+ typeCode == SelectField.MESSAGE_TYPE_CODE && hopElements.size() == 3,
+ "Indexed qualifier hop must be a repeated message or a message-valued map field: %s",
+ hopElements);
+ return SelectField.create(
+ fieldNumber, fieldName, typeCode, /* defaultValue= */ ImmutableList.of());
+ }
+
private static SelectField parseRepeatedLeaf(
long fieldNumber, String fieldName, long typeCode, CelExpr dummyExpr) {
checkArgument(
@@ -199,8 +256,7 @@ private static SelectField parseSingularLeaf(
return SelectField.create(fieldNumber, fieldName, typeCode, defaultValue, protoTypeName);
}
- private static ImmutableList unpackHasFieldFields(CelExpr qualifiersExpr) {
- ImmutableList elements = unpackQualifierHops(qualifiersExpr);
+ private static ImmutableList unpackHasFieldFields(ImmutableList elements) {
ImmutableList.Builder fieldsBuilder =
ImmutableList.builderWithExpectedSize(elements.size());
for (CelExpr hopExpr : elements) {
@@ -429,9 +485,46 @@ private OptimizedSelectQualifier(
}
}
+ /** Indexes a repeated or map field's value by a constant list index or map key. */
+ @Immutable
+ private static final class IndexQualifier implements Qualifier {
+ @SuppressWarnings("Immutable") // Immutable CEL constant.
+ private final Object index;
+
+ private final RuntimeEquality runtimeEquality;
+ private final CelValueConverter celValueConverter;
+
+ @Override
+ public Object value() {
+ return index;
+ }
+
+ // The operand is the protobuf field's list or map traversal target, so its elements are indexed
+ // in place rather than unwrapped first as the generic index operator's arguments are.
+ @Override
+ @SuppressWarnings("unchecked") // Map keys are compared as CEL values.
+ public Object qualify(Object operand) {
+ Object element =
+ operand instanceof Map
+ ? runtimeEquality.indexMap((Map) operand, index)
+ : RuntimeHelpers.indexList((List) operand, (Long) index);
+ return celValueConverter.toTraversalTarget(element);
+ }
+
+ private IndexQualifier(
+ Object index, RuntimeEquality runtimeEquality, CelValueConverter celValueConverter) {
+ this.index = checkNotNull(index);
+ this.runtimeEquality = checkNotNull(runtimeEquality);
+ this.celValueConverter = checkNotNull(celValueConverter);
+ }
+ }
+
private OptimizedSelectPlanner(
- AttributeFactory attributeFactory, CelValueConverter celValueConverter) {
+ AttributeFactory attributeFactory,
+ CelValueConverter celValueConverter,
+ RuntimeEquality runtimeEquality) {
this.attributeFactory = checkNotNull(attributeFactory);
this.celValueConverter = checkNotNull(celValueConverter);
+ this.runtimeEquality = checkNotNull(runtimeEquality);
}
}
diff --git a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java
index 3782d1860..4314e66ad 100644
--- a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java
+++ b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java
@@ -935,6 +935,6 @@ private ProgramPlanner(
this.attributeFactory =
AttributeFactory.newAttributeFactory(container, typeProvider, celValueConverter);
this.optimizedSelectPlanner =
- OptimizedSelectPlanner.create(this.attributeFactory, celValueConverter);
+ OptimizedSelectPlanner.create(this.attributeFactory, celValueConverter, runtimeEquality);
}
}
diff --git a/runtime/src/test/java/dev/cel/runtime/CelAttributePatternTest.java b/runtime/src/test/java/dev/cel/runtime/CelAttributePatternTest.java
index de265cc77..bb7eab9a1 100644
--- a/runtime/src/test/java/dev/cel/runtime/CelAttributePatternTest.java
+++ b/runtime/src/test/java/dev/cel/runtime/CelAttributePatternTest.java
@@ -18,6 +18,7 @@
import static org.junit.Assert.assertThrows;
import com.google.common.collect.ImmutableList;
+import com.google.common.primitives.UnsignedLong;
import dev.cel.runtime.CelAttribute.Qualifier;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -244,6 +245,36 @@ public void isPartialMatch_doesNotMatch() {
.isFalse();
}
+ @Test
+ public void isMatch_intAndUintQualifiersWithSameValue_match() {
+ CelAttributePattern intPattern =
+ CelAttributePattern.create("identifier").qualify(Qualifier.ofInt(1));
+ CelAttributePattern uintPattern =
+ CelAttributePattern.create("identifier").qualify(Qualifier.ofUint(1));
+ CelAttribute intAttribute = CelAttribute.create("identifier").qualify(Qualifier.ofInt(1));
+ CelAttribute uintAttribute = CelAttribute.create("identifier").qualify(Qualifier.ofUint(1));
+
+ assertThat(intPattern.isMatch(uintAttribute)).isTrue();
+ assertThat(uintPattern.isMatch(intAttribute)).isTrue();
+ assertThat(intPattern.isPartialMatch(uintAttribute)).isTrue();
+ assertThat(uintPattern.isPartialMatch(intAttribute)).isTrue();
+ }
+
+ @Test
+ public void isMatch_negativeIntAndUintWithSameBits_doNotMatch() {
+ CelAttributePattern intPattern =
+ CelAttributePattern.create("identifier").qualify(Qualifier.ofInt(-1));
+ CelAttributePattern uintPattern =
+ CelAttributePattern.create("identifier")
+ .qualify(Qualifier.ofUint(UnsignedLong.fromLongBits(-1)));
+ CelAttribute intAttribute = CelAttribute.create("identifier").qualify(Qualifier.ofInt(-1));
+ CelAttribute uintAttribute =
+ CelAttribute.create("identifier").qualify(Qualifier.ofUint(UnsignedLong.fromLongBits(-1)));
+
+ assertThat(intPattern.isMatch(uintAttribute)).isFalse();
+ assertThat(uintPattern.isMatch(intAttribute)).isFalse();
+ }
+
@Test
public void simplify_simplifies() {
assertThat(
diff --git a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java
index 5fbf0b1a6..bfec9551c 100644
--- a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java
+++ b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java
@@ -46,6 +46,7 @@
import com.google.protobuf.Value;
import com.google.testing.junit.testparameterinjector.TestParameter;
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
+import com.google.testing.junit.testparameterinjector.TestParameters;
import dev.cel.bundle.Cel;
import dev.cel.bundle.CelFactory;
import dev.cel.common.CelAbstractSyntaxTree;
@@ -762,6 +763,19 @@ public void select_renamedMapField_indexing() throws Exception {
assertThat(result).isEqualTo(100L);
}
+ @Test
+ @TestParameters({
+ "{expression: 'has(msg.repeated_nested_message[1].bb)', expected: true}",
+ "{expression: 'has(msg.map_int64_nested_type[1].child.payload)', expected: true}",
+ "{expression: 'has(msg.map_int64_nested_type[2].child.payload)', expected: false}"
+ })
+ public void has_throughUnknownRepeatedAndMapIndices_decodesFromWireBytes(
+ String expression, boolean expected) throws Exception {
+ Object result = eval(expression, POPULATED_SERVER_MESSAGE);
+
+ assertThat(result).isEqualTo(expected);
+ }
+
@Test
public void has_renamedRepeatedField_emptyPackedWireBytes_returnsFalse() throws Exception {
// Evaluated via newOlderClassV1Runtime() on nested_msg.payload so payload is a wire-backed
diff --git a/runtime/src/test/java/dev/cel/runtime/planner/OptimizedSelectPlannerTest.java b/runtime/src/test/java/dev/cel/runtime/planner/OptimizedSelectPlannerTest.java
index 9cf457cfe..ecb6d2052 100644
--- a/runtime/src/test/java/dev/cel/runtime/planner/OptimizedSelectPlannerTest.java
+++ b/runtime/src/test/java/dev/cel/runtime/planner/OptimizedSelectPlannerTest.java
@@ -783,15 +783,6 @@ private enum ScalarDummyMismatchTestCase {
private final ImmutableList hopElements;
private final CelExpr dummy;
- private static CelExpr mapEntrySpec(long keyTypeCode, long valueTypeCode) {
- return CelExpr.ofList(
- 8L,
- ImmutableList.of(
- CelExpr.ofConstant(9L, CelConstant.ofValue(keyTypeCode)),
- CelExpr.ofConstant(10L, CelConstant.ofValue(valueTypeCode))),
- ImmutableList.of());
- }
-
ScalarDummyMismatchTestCase(ImmutableList hopElements, CelExpr dummy) {
this.hopElements = hopElements;
this.dummy = dummy;
@@ -930,6 +921,105 @@ public void plan_invalidAst_messageStructWithEmptyMessageName_throwsEvaluationEx
assertThat(e).hasMessageThat().contains("Protobuf message type name must not be empty");
}
+ @SuppressWarnings("ImmutableEnumChecker") // Test only
+ private enum MalformedIndexHopTestCase {
+ LEADING_INDEX(
+ ImmutableList.of(constantHop(0L), fieldHop(1L, "bb", 5L)),
+ "Index qualifier hop must be between field hops"),
+ TRAILING_INDEX(
+ ImmutableList.of(fieldHop(51L, "repeated_nested_message", 11L), constantHop(0L)),
+ "Index qualifier hop must be between field hops"),
+ CONSECUTIVE_INDICES(
+ ImmutableList.of(
+ fieldHop(51L, "repeated_nested_message", 11L),
+ constantHop(0L),
+ constantHop(0L),
+ fieldHop(1L, "bb", 5L)),
+ "Index qualifier hop must be between field hops"),
+ SCALAR_FIELD_INDEXED(
+ ImmutableList.of(fieldHop(2L, "single_int64", 3L), constantHop(0L), fieldHop(1L, "bb", 5L)),
+ "Indexed qualifier hop must be a repeated message or a message-valued map field"),
+ SCALAR_VALUED_MAP_INDEXED(
+ ImmutableList.of(
+ mapHop(61L, "map_string_string", 9L, 9L), constantHop("k"), fieldHop(1L, "bb", 5L)),
+ "Indexed map qualifier hop must have MESSAGE values (11), found: 9"),
+ NON_INT_LIST_INDEX(
+ ImmutableList.of(
+ fieldHop(51L, "repeated_nested_message", 11L),
+ constantHop("0"),
+ fieldHop(1L, "bb", 5L)),
+ "is incompatible with type code 3"),
+ MAP_KEY_KIND_MISMATCH(
+ ImmutableList.of(
+ mapHop(95L, "map_int64_message", 3L, 11L),
+ constantHop(UnsignedLong.ONE),
+ fieldHop(1L, "bb", 5L)),
+ "is incompatible with type code 3");
+
+ private final ImmutableList hops;
+ private final String expectedMessageSubstring;
+
+ MalformedIndexHopTestCase(ImmutableList hops, String expectedMessageSubstring) {
+ this.hops = hops;
+ this.expectedMessageSubstring = expectedMessageSubstring;
+ }
+ }
+
+ @Test
+ public void plan_invalidAst_malformedIndexHop_throwsEvaluationException(
+ @TestParameter MalformedIndexHopTestCase testCase) {
+ CelAbstractSyntaxTree ast =
+ CelAbstractSyntaxTree.newParsedAst(
+ CelExpr.ofCall(
+ 1L,
+ OptimizedSelectPlanner.CEL_ATTRIBUTE_FUNCTION_NAME,
+ ImmutableList.of(
+ CelExpr.ofIdent(2L, "msg"),
+ CelExpr.ofList(3L, testCase.hops, ImmutableList.of()),
+ CelExpr.ofConstant(4L, CelConstant.ofValue(0L)))),
+ SELECT_OPTIMIZED_SOURCE);
+
+ CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast));
+
+ assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
+ assertThat(e).hasMessageThat().contains(testCase.expectedMessageSubstring);
+ }
+
+ private static CelExpr fieldHop(long fieldNumber, String fieldName, long typeCode) {
+ return CelExpr.ofList(
+ 0L,
+ ImmutableList.of(
+ CelExpr.ofConstant(0L, CelConstant.ofValue(fieldNumber)),
+ CelExpr.ofConstant(0L, CelConstant.ofValue(fieldName)),
+ CelExpr.ofConstant(0L, CelConstant.ofValue(typeCode))),
+ ImmutableList.of());
+ }
+
+ private static CelExpr mapHop(
+ long fieldNumber, String fieldName, long keyTypeCode, long valueTypeCode) {
+ return CelExpr.ofList(
+ 0L,
+ ImmutableList.of(
+ CelExpr.ofConstant(0L, CelConstant.ofValue(fieldNumber)),
+ CelExpr.ofConstant(0L, CelConstant.ofValue(fieldName)),
+ CelExpr.ofConstant(0L, CelConstant.ofValue(-1L)),
+ mapEntrySpec(keyTypeCode, valueTypeCode)),
+ ImmutableList.of());
+ }
+
+ private static CelExpr mapEntrySpec(long keyTypeCode, long valueTypeCode) {
+ return CelExpr.ofList(
+ 8L,
+ ImmutableList.of(
+ CelExpr.ofConstant(9L, CelConstant.ofValue(keyTypeCode)),
+ CelExpr.ofConstant(10L, CelConstant.ofValue(valueTypeCode))),
+ ImmutableList.of());
+ }
+
+ private static CelExpr constantHop(Object index) {
+ return CelExpr.ofConstant(0L, CelConstant.ofObjectValue(index));
+ }
+
private static CelAbstractSyntaxTree optimizeSelectAst(String expression) throws Exception {
CelAbstractSyntaxTree ast = CEL.compile(expression).getAst();
CelAbstractSyntaxTree optimizedAst = SELECT_OPTIMIZER.optimize(ast);
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 4e006cdd4..dbe0a2f1d 100644
--- a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java
+++ b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java
@@ -47,6 +47,7 @@
import dev.cel.common.ast.CelExpr.CelComprehension;
import dev.cel.common.exceptions.CelDivideByZeroException;
import dev.cel.common.exceptions.CelInvalidArgumentException;
+import dev.cel.common.exceptions.CelRuntimeException;
import dev.cel.common.internal.CelDescriptorPool;
import dev.cel.common.internal.DefaultDescriptorPool;
import dev.cel.common.internal.DefaultMessageFactory;
@@ -769,7 +770,7 @@ public void plan_call_index_withUnknownTarget() throws Exception {
assertThat(result)
.isEqualTo(
CelUnknownSet.create(
- ImmutableSet.of(CelAttribute.create("unk_list")), ImmutableSet.of(1L)));
+ ImmutableSet.of(CelAttribute.create("unk_list")), ImmutableSet.of(2L)));
}
@Test
@@ -811,6 +812,114 @@ public void plan_call_index_withMultipleUnknowns_mergesUnknowns() throws Excepti
ImmutableSet.of(1L, 3L)));
}
+ @Test
+ public void plan_call_constantIndex_withPartialVarsOnOtherElement_returnsValue()
+ throws Exception {
+ CelAbstractSyntaxTree ast = compile("msg.repeated_nested_message[0].bb");
+ Program program = PLANNER.plan(ast);
+ TestAllTypes msg =
+ TestAllTypes.newBuilder()
+ .addRepeatedNestedMessage(NestedMessage.newBuilder().setBb(1))
+ .addRepeatedNestedMessage(NestedMessage.newBuilder().setBb(2))
+ .build();
+
+ Object result =
+ program.eval(
+ PartialVars.of(
+ ImmutableMap.of("msg", msg),
+ CelAttributePattern.fromQualifiedIdentifier("msg.repeated_nested_message")
+ .qualify(CelAttribute.Qualifier.ofInt(1))));
+
+ assertThat(result).isEqualTo(1L);
+ }
+
+ @Test
+ public void plan_call_constantIndex_withPartialVarsOnIndexedElement_returnsUnknown()
+ throws Exception {
+ CelAbstractSyntaxTree ast = compile("msg.repeated_nested_message[0].bb");
+ Program program = PLANNER.plan(ast);
+ CelAttribute indexedElement =
+ CelAttribute.fromQualifiedIdentifier("msg.repeated_nested_message")
+ .qualify(CelAttribute.Qualifier.ofInt(0));
+
+ Object result =
+ program.eval(
+ PartialVars.of(
+ CelAttributePattern.fromQualifiedIdentifier("msg.repeated_nested_message")
+ .qualify(CelAttribute.Qualifier.ofInt(0))));
+
+ assertThat(result)
+ .isEqualTo(CelUnknownSet.create(ImmutableSet.of(indexedElement), ImmutableSet.of(5L)));
+ }
+
+ @SuppressWarnings("ImmutableEnumChecker") // Test only
+ private enum ConstantIndexUnknownTestCase {
+ UINT_INDEX_MATCHES_INT_PATTERN(
+ "dyn_var[1u]",
+ CelAttribute.Qualifier.ofInt(1),
+ CelAttribute.create("dyn_var").qualify(CelAttribute.Qualifier.ofUint(1))),
+ INT_INDEX_MATCHES_UINT_PATTERN(
+ "dyn_var[1]",
+ CelAttribute.Qualifier.ofUint(1),
+ CelAttribute.create("dyn_var").qualify(CelAttribute.Qualifier.ofInt(1))),
+ BOOL_INDEX(
+ "dyn_var[true]",
+ CelAttribute.Qualifier.ofBool(true),
+ CelAttribute.create("dyn_var").qualify(CelAttribute.Qualifier.ofBool(true))),
+ // A double isn't an attribute qualifier, so the index isn't folded into the attribute.
+ DOUBLE_INDEX_STOPS_AT_INDEX(
+ "dyn_var[1.0]", CelAttribute.Qualifier.ofInt(1), CelAttribute.create("dyn_var"));
+
+ private final String expression;
+ private final CelAttribute.Qualifier patternIndex;
+ private final CelAttribute expectedAttribute;
+
+ ConstantIndexUnknownTestCase(
+ String expression, CelAttribute.Qualifier patternIndex, CelAttribute expectedAttribute) {
+ this.expression = expression;
+ this.patternIndex = patternIndex;
+ this.expectedAttribute = expectedAttribute;
+ }
+ }
+
+ @Test
+ public void plan_call_constantIndex_withPartialVars_returnsUnknown(
+ @TestParameter ConstantIndexUnknownTestCase testCase) throws Exception {
+ CelAbstractSyntaxTree ast = compile(testCase.expression);
+ Program program = PLANNER.plan(ast);
+
+ CelUnknownSet result =
+ (CelUnknownSet)
+ program.eval(
+ PartialVars.of(
+ CelAttributePattern.create("dyn_var").qualify(testCase.patternIndex)));
+
+ assertThat(result.attributes()).containsExactly(testCase.expectedAttribute);
+ }
+
+ @Test
+ public void plan_call_constantIndexOnEnumConstant_throws() throws Exception {
+ CelAbstractSyntaxTree ast = CEL_COMPILER.parse("GlobalEnum.GAR[0]").getAst();
+ Program program = PLANNER.plan(ast);
+
+ assertThrows(CelEvaluationException.class, program::eval);
+ }
+
+ @Test
+ public void plan_call_constantIndex_outOfBounds_localizesErrorToIndex() throws Exception {
+ CelAbstractSyntaxTree ast = compile("msg.repeated_nested_message[1].bb");
+ Program program = PLANNER.plan(ast);
+
+ CelEvaluationException e =
+ assertThrows(
+ CelEvaluationException.class,
+ () -> program.eval(ImmutableMap.of("msg", TestAllTypes.getDefaultInstance())));
+
+ assertThat(e)
+ .hasMessageThat()
+ .isEqualTo("evaluation error at