jayzhan211 commented on code in PR #25339:
URL: https://github.com/apache/datafusion/pull/25339#discussion_r4046914195
##########
datafusion/optimizer/src/decorrelate_predicate_subquery.rs:
##########
@@ -584,25 +583,12 @@ fn build_join(
sub_query_alias.clone()
};
- let mark_filter_is_hashable_only =
- if join_type == JoinType::LeftMark && in_predicate_opt.is_some() {
- let (_, residual_filter) = split_eq_and_noneq_join_predicate(
- join_filter.clone(),
- left.schema(),
- right_projected.schema(),
- )?;
- residual_filter.is_none()
- } else {
- false
- };
-
// For scalar NOT IN mark joins, propagate null-aware semantics into
the
- // nullable mark column when the predicate can be implemented by hash
keys.
- // Non-equality correlated filters stay on the legacy path because
hash join
- // execution cannot mark UNKNOWN candidates for residual predicates.
+ // nullable mark column. A non-equality correlation stays behind as a
+ // join filter, which the hash join also applies when it decides
+ // whether a NULL makes the mark UNKNOWN.
let null_aware = join_type == JoinType::LeftMark
Review Comment:
This change also makes plain `IN` (not just `NOT IN`) null-aware when the
subquery has a non-equality correlation; see the four changed plans in
`subquery.slt`. In `WHERE a OR x IN (...)` the filter drops the row whether the
mark is NULL or FALSE, so base was already correct for these queries. The
null-aware plan only adds cost: it pins the outer table as the `CollectLeft`
build side, cannot swap, and evaluates the join filter for every (build row ×
NULL probe row) pair.
Measured (release-nonlto, 100k × 100k, same result 9900 on both): base 1–2
ms → PR 7.4–8.1 s. At 30k rows it takes 0.57 s, so it is quadratic. The plan
changes from `RightMark` to `LeftMark ... null_aware`.
Repro:
```sql
CREATE TABLE so AS SELECT value AS id_n0, value % 1000 AS z FROM range(0,
100000);
CREATE TABLE si AS SELECT CASE WHEN value % 2 = 0 THEN NULL ELSE value * 2
END AS id_n50, value % 1000 AS z FROM range(0, 100000);
SELECT count(*) FROM so o
WHERE o.z > 900 OR o.id_n0 IN (SELECT i.id_n50 FROM si i WHERE i.z > o.z +
990);
```
Fix: only make the mark join null-aware when a NULL mark can behave
differently from FALSE. That is never the case when the subquery is a
non-negated `IN`/`EXISTS` sitting directly under `AND`/`OR` in a `WHERE`
conjunct. A helper to detect that:
```rs
/// True when every subquery in `expr` is a non-negated `IN`/`EXISTS` reached
/// only through AND/OR. A filter treats a NULL mark like FALSE there, so the
/// mark join does not need to be null-aware.
fn subqueries_only_positive(expr: &Expr) -> bool {
match expr {
Expr::BinaryExpr(BinaryExpr {
left,
op: Operator::And | Operator::Or,
right,
}) => subqueries_only_positive(left) &&
subqueries_only_positive(right),
Expr::InSubquery(InSubquery { negated, .. }) => !negated,
Expr::Exists(Exists { negated, .. }) => !negated,
other => !has_subquery(other),
}
}
```
Call it on each conjunct in the `SubqueryPredicate::Embedded` arm and pass
the result down `rewrite_inner_subqueries` → `mark_join` → `build_join` as a
new `bool` (say `needs_null_aware_mark`). Callers outside a `Filter` pass
`true`. Then:
```diff
let null_aware = join_type == JoinType::LeftMark
&& in_predicate_opt.is_some()
+ && needs_null_aware_mark
&& join_keys_may_be_null(
```
`NOT IN`, `NOT (x IN ...)`, `(x IN ...) IS NULL`, `CASE` and projected marks
stay null-aware, so the fix in this PR is unaffected. The four `subquery.slt`
plan diffs should revert; please add an `EXPLAIN` test that pins the
positive-`IN` plan as non-null-aware
--
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]
