google / cel-java

Fast, portable, non-Turing complete expression evaluation with gradual typing (Java)
https://cel.dev
Apache License 2.0
146 stars 17 forks source link

[BUG] ConstantFoldingOptimizer doesn't work for some expressions #462

Open bugs84 opened 1 day ago

bugs84 commented 1 day ago

Describe the bug ConstantFoldingOptimizer doesn't work for some expressions. e.g. "my_var in ['H', 'O']"

To Reproduce Clone this repository, that reproduce the bug. https://github.com/bugs84/CelOptimizerIssue

Current issue Program ends with following exception:

Exception in thread "main" java.lang.NoSuchMethodError: 'com.google.protobuf.Internal$LongList dev.cel.expr.UnknownSet.mutableCopy(com.google.protobuf.Internal$LongList)'
    at dev.cel.expr.UnknownSet.access$600(UnknownSet.java:14)
    at dev.cel.expr.UnknownSet$Builder.ensureExprsIsMutable(UnknownSet.java:456)
    at dev.cel.expr.UnknownSet$Builder.addAllExprs(UnknownSet.java:539)
    at dev.cel.runtime.InterpreterUtil.createUnknownExprValue(InterpreterUtil.java:95)
    at dev.cel.runtime.InterpreterUtil.createUnknownExprValue(InterpreterUtil.java:84)
    at dev.cel.runtime.InterpreterUtil.valueOrUnknown(InterpreterUtil.java:146)
    at dev.cel.runtime.RuntimeUnknownResolver.resolveSimpleName(RuntimeUnknownResolver.java:114)
    at dev.cel.runtime.DefaultInterpreter$ExecutionFrame.resolveSimpleName(DefaultInterpreter.java:975)
    at dev.cel.runtime.DefaultInterpreter$ExecutionFrame.access$200(DefaultInterpreter.java:936)
    at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.resolveIdent(DefaultInterpreter.java:274)
    at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.evalIdent(DefaultInterpreter.java:262)
    at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.evalInternal(DefaultInterpreter.java:193)
    at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.evalCall(DefaultInterpreter.java:387)
    at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.evalInternal(DefaultInterpreter.java:199)
    at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.evalTrackingUnknowns(DefaultInterpreter.java:179)
    at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.eval(DefaultInterpreter.java:170)
    at dev.cel.runtime.CelRuntime$Program.evalInternal(CelRuntime.java:146)
    at dev.cel.runtime.CelRuntime$Program.evalInternal(CelRuntime.java:121)
    at dev.cel.runtime.CelRuntime$Program.evalInternal(CelRuntime.java:116)
    at dev.cel.runtime.CelRuntime$Program.eval(CelRuntime.java:49)
    at dev.cel.common.ast.CelExprUtil.evaluateExpr(CelExprUtil.java:42)
    at dev.cel.optimizer.optimizers.ConstantFoldingOptimizer.maybeFold(ConstantFoldingOptimizer.java:251)
    at dev.cel.optimizer.optimizers.ConstantFoldingOptimizer.optimize(ConstantFoldingOptimizer.java:113)
    at dev.cel.optimizer.CelOptimizerImpl.optimize(CelOptimizerImpl.java:45)
    at HelloWorld.run(HelloWorld.java:62)
    at HelloWorld.main(HelloWorld.java:73)
l46kok commented 1 day ago

I was able to reproduce this. My suspicion is that something about the protobuf upgrade to 4.28.0 is causing this issue, and that the binary compatibility may not have been fully restored as stated in https://github.com/protocolbuffers/protobuf/issues/17247. Either that, or the dependent protos (ExprValue from proto-google-common-protos, used to represent unknowns in CEL) is not fully protobuf v4 compatible.

The workaround is to downgrade CEL to 0.6.0 as of now. I'll look into this further.

l46kok commented 22 hours ago

This is an issue on any code path in CEL that references the Builder's methods on protobuf messages. The earlier release of Protobuf v4 have removed GeneratedMessageV3, and shims were added afterwards to restore ABI compatibility in 4.27.x, but not fully. Particularly, any generated messages produced before protoc version 3.25.x are no longer compatible with Protobuf V4. Bazel's built-in proto toolchain (@com_google_protobuf//:protoc) happens to use 3.21. This unfortunately led to publishing a JAR with incompatible generated messages.

In short, CEL is not compatible with protobuf v4. The published JAR for 0.7.1 forces protobuf v4 as a dependency so it should not be used. I'll try to release a fix soon.

bugs84 commented 22 hours ago

Thank you.

Just note: Downgrade to CEL 0.6.0 - has different issue NoClassDefFoundError: com/google/rpc/StatusProto But it doesn't matter. For us is now enough to comment out ConstantFoldingOptimizer. And wait for proper fix. Thank you again!

l46kok commented 21 hours ago

Oh for that one, you just need to bring in proto-google-common-protos from maven. Not sure why that's not being resolved automatically in gradle, but that's a separate issue.

Btw, the issue in 0.7.1 is not just specific to constant folding. You'll run into issues in other places where an unknown value needs to be referenced at all internally.