DonnyZone commented on code in PR #3203:
URL: https://github.com/apache/calcite/pull/3203#discussion_r1209127755
##########
linq4j/src/main/java/org/apache/calcite/linq4j/tree/OptimizeShuttle.java:
##########
@@ -164,6 +169,9 @@ && eq(cmp.expression1, expression2)) {
Expression expression1) {
//
Expression result;
+ if (expression0 instanceof ParameterExpression) {
Review Comment:
I get your point now. The pattern to optimize is `Integer x = (Integer)
null`. But it is not safe to check the `ParameterExpression` here. For example
(`x || null `compilation error):
```
@Test void testOptimizeBinaryNullCasting2() {
// Boolean x;
ParameterExpression x = Expressions.variable(Boolean.class, "x");
ParameterExpression y = Expressions.variable(Boolean.class, "y");
// x = Boolean.valueOf(true)
BinaryExpression xt = Expressions.assign(x,
new ConstantExpression(Boolean.class, true));
System.out.println(xt);
// Boolean y = x || (Boolean) null;
BinaryExpression yt = Expressions.assign(y,
Expressions.orElse(x,
new ConstantExpression(Boolean.class, null)));
assert(optimize(yt).contains("x || (Boolean) null"));
}
```
I suggest to check the node type.
```
if (binary.getNodeType() == ExpressionType.Assign) {
expression1 = skipNullCast(expression1);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
