peter-toth commented on code in PR #60162:
URL: https://github.com/apache/spark/pull/60162#discussion_r4490602401
##########
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:
**Finding 4.** After #60119, the remaining cost comes from constraints that
can fail only in ANSI mode, such as `CAST(r.s AS INT) > 0` or `r.i * 2 > 1000`
on a join key. On most data these never fail. The head drops them anyway,
together with the partition pruning they gave the other side.
An inferred filter only drops rows that can't reach the output anyway. So it
is safe for it to keep a row it can't decide. That allows a copy that can't
fail:
- in the copy, turn the ANSI arithmetic and casts into their TRY forms,
which return NULL instead of failing,
- add the copy as `<copy> IS NOT FALSE`, so that a NULL keeps the row.
For example, `l JOIN r ON l.s = r.s WHERE CAST(r.s AS INT) > 0` would give
`l` the filter `(TRY_CAST(l.s AS INT) > 0) IS NOT FALSE`. That would prune the
partitions of `l` as on the base, and an `'abc'` in `l.s` would not fail the
query.
The counter-arguments:
- **Coverage:** not every expression has a TRY form that can't fail.
`IntegralDivide` and `Pmod` still raise on a zero divisor in TRY mode. UDFs,
`raise_error` and `assert_true` stay out.
- **No generic wrapper:** wrapping the copy in `TryEval` doesn't work
instead. As the description of #60119 notes, subexpression elimination can move
a part that fails out of it.
- **Pushdown:** data source pushdown doesn't come back. Data Source V2
doesn't get a TRY cast, and Parquet never got a cast or arithmetic on a column.
- **Complexity:** it adds more logic to a subtle rule. I haven't prototyped
or measured it.
Could this be a follow-up, or does it look worth doing here?
--
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]
