ulysses-you commented on code in PR #40446:
URL: https://github.com/apache/spark/pull/40446#discussion_r1139980442
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/EquivalentExpressions.scala:
##########
@@ -128,10 +131,23 @@ class EquivalentExpressions {
// There are some special expressions that we should not recurse into all of
its children.
// 1. CodegenFallback: it's children will not be used to generate code
(call eval() instead)
// 2. ConditionalExpression: use its children that will always be
evaluated.
- private def childrenToRecurse(expr: Expression): Seq[Expression] = expr
match {
- case _: CodegenFallback => Nil
- case c: ConditionalExpression => c.alwaysEvaluatedInputs
- case other => other.children
+ private def childrenToRecurse(expr: Expression): Seq[Expression] = {
+ val alwaysEvaluated = expr match {
+ case _: CodegenFallback => Nil
+ case c: ConditionalExpression => c.alwaysEvaluatedInputs
+ case other => other.children
+ }
+ if (shortcut) {
+ // The subexpression may not need to eval even if it appears more than
once.
+ // e.g., `if(or(a, and(b, b)))`, the expression `b` would be skipped if
`a` is true.
+ alwaysEvaluated.map {
+ case and: And => and.left
+ case or: Or => or.left
+ case other => other
+ }
Review Comment:
It's a kind of lazy evaluation. I remember before that pr attempts
https://github.com/apache/spark/pull/32977 , and there is some issues @viirya
memtioned
https://github.com/apache/spark/pull/32977#pullrequestreview-690266902.
It seems the main issue is that, the method will go to large if we make each
common subexpression evaluation lazy. Something like:
```
def common_subexpression_1() {
if (isnull) {
// evaluate
} else {
// return exists value
}
}
```
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
