jayzhan211 commented on code in PR #26942:
URL: https://github.com/apache/datafusion/pull/26942#discussion_r4504898575
##########
datafusion/optimizer/src/scalar_subquery_to_join.rs:
##########
@@ -345,6 +345,24 @@ fn build_join(
.collected_count_expr_map
.get(&decorrelated_subquery)
.cloned();
+
+ // A pulled up `HAVING` is only applied through the count bug compensation
+ // below, which skips the expressions that are NULL on empty input. Keep
+ // the subquery correlated rather than drop the `HAVING` for its output.
+ // <https://github.com/apache/datafusion/issues/26505>
+ if pull_up.pull_up_having_expr.is_some() {
Review Comment:
Since #26723 merged, `main` already computes this lookup in `build_join` as
`output_needs_compensation`, so after a rebase the same check would be done
twice. Could you rebase and gate on that variable instead? It treats a missing
output as `false`, so the `else` branch here (which no test reaches) is covered
too. I ran this on the PR merged with `main`: `subquery.slt` and the optimizer
unit tests pass, and clippy is clean.
```diff
- if pull_up.pull_up_having_expr.is_some() {
- let output_name = subquery_plan.schema().field(0).name();
- let Some(result) = collected_count_expr_map
- .as_ref()
- .and_then(|expr_map| expr_map.get(output_name))
- else {
- return Ok(None);
- };
- if evaluates_to_null(result.clone(), result.column_refs())? {
- return Ok(None);
- }
- }
...
};
+ if pull_up.pull_up_having_expr.is_some() && !output_needs_compensation {
+ return Ok(None);
+ }
if output_needs_compensation &&
!pull_up.having_like_join_filters.is_empty() {
```
--
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]
