sigmod commented on a change in pull request #32280:
URL: https://github.com/apache/spark/pull/32280#discussion_r618944025
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
##########
@@ -740,8 +751,10 @@ object NullPropagation extends Rule[LogicalPlan] {
case _ => false
}
- def apply(plan: LogicalPlan): LogicalPlan = plan transform {
- case q: LogicalPlan => q transformExpressionsUp {
+ def apply(plan: LogicalPlan): LogicalPlan = plan.transformWithPruning(
+ _.containsAnyPattern(NULL_CHECK, NULL_LITERAL, COUNT, CAST), ruleId) {
Review comment:
>> seems no good solution for avoiding regressions except for asking
developers to add more unit tests
- For an existing rule that we're adding this pruning, yes, we need to be
careful, particularly when a rule has complex case patterns, although many
rules are simple and intuitive.
- For a new rule or a new case branch in an existing rule, it seems that the
regression risk doesn't change much? if a case pattern branch is not covered by
unit tests, there's also a regression risk even without bits pruning, e.g., the
case pattern could be wrong too.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
