dongjoon-hyun commented on code in PR #60162:
URL: https://github.com/apache/spark/pull/60162#discussion_r4495747529
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -2317,17 +2317,40 @@ object InferFiltersFromConstraints extends
Rule[LogicalPlan]
}
private def inferNewFilter(plan: LogicalPlan, constraints: ExpressionSet):
LogicalPlan = {
- val newPredicates = constraints
+ val newPredicates = (constraints
.union(constructIsNotNullConstraints(constraints, plan.output))
.filter { c =>
c.references.nonEmpty && c.references.subsetOf(plan.outputSet) &&
c.deterministic
- } -- plan.constraints
+ } -- plan.constraints).toSeq.filter(canBeFilter)
if (newPredicates.isEmpty) {
plan
} else {
Filter(newPredicates.reduce(And), plan)
}
}
+
+ /**
+ * A constraint only says what holds on the rows an operator outputs. As a
filter, it also runs
+ * on rows and values the query may never evaluate it on, e.g. `10 / x > 0`
from `x = y AND
+ * 10 / y > 0` on a row where `x` is 0 and `y` is not, so only a constraint
that cannot fail
+ * becomes a filter. Constant parts count as what they evaluate to, as they
are folded right
+ * after this rule, and an uncorrelated subquery as a constant, as it runs
once whichever rows
+ * use it. Callers pass the original expressions, because
[[ExpressionSet.filter]] tests the
+ * canonicalized ones.
+ */
+ private def canBeFilter(constraint: Expression): Boolean = {
Review Comment:
Thanks for the detailed review, and for measuring this against #60119.
I would like to keep the two changes independent. This PR only gates the two
places in `InferFiltersFromConstraints` that turn a constraint into a filter.
#60119 widens what `ExprUtils.canEvaluateUnconditionally` accepts, which is
shared with other rules, so it should be reviewed and tested on its own. They
can land in either order, and once #60119 is merged the filters in your table
are inferred again without any change here. For that reason I would rather not
copy part of its allowlist into this PR.
Until then, `LIKE 'a%'`, `year(d) = ...` and ANSI-off arithmetic are not
inferred on the other side of a join, and
`spark.sql.legacy.inferFailableFiltersFromConstraints` restores the previous
behavior. I will fix the description, the migration guide and the config doc,
which currently say that only a filter that can fail is skipped.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
