Minor: Add PullUpCorrelatedExpr::new and improve documentation
#10500
Conversation
| pub can_pull_up: bool, | ||
| // indicate whether need to handle the Count bug during the pull up process | ||
| /// Do we need to handle the Count bug during the pull up process |
There was a problem hiding this comment.
what/who is the "count bug"? Is this a hotfix for an actual issue?
There was a problem hiding this comment.
I did a quick look and couldn't figure it out
It looks like it was introduced in #6457. @mingmwang or @jackwener do you remember what the "count bug is" and if it has a ticket?
There was a problem hiding this comment.
I checked the param, if we dont use it, the correlated subquery fails
Running "subquery.slt"
External error: query result mismatch:
[SQL] SELECT t1_id, (SELECT count(*) FROM t2 WHERE t2.t2_int = t1.t1_int) from t1
[Diff] (-expected|+actual)
11 1
- 22 0
+ 22 NULL
33 3
- 44 0
+ 44 NULL
at test_files/subquery.slt:763
Looks like its related if there is no match in correlated subq the count should be 0 instead of NULL
There was a problem hiding this comment.
it should be introduced by @mingmwang
| pull_up_having_expr: None, | ||
| }; | ||
| let mut pull_up = PullUpCorrelatedExpr::new() | ||
| .with_in_predicate_opt(in_predicate_opt.clone()) |
There was a problem hiding this comment.
these were the two fields that are set differently which I think is much clearer now
Co-authored-by: Oleks V <[email protected]>
Thank you -- I will file a ticket to track the issue and leave a note in the comments |
…che#10500) * Minor `PullUpCorrelatedExpr::new` and add documentation * clippy * Update datafusion/optimizer/src/decorrelate.rs Co-authored-by: Oleks V <[email protected]> * Add ticket reference to count bug --------- Co-authored-by: Oleks V <[email protected]>
Which issue does this PR close?
Closes #10294
Rationale for this change
While working on #10489 I spent some time studying
PullUpCorrelatedExpras it wasn't clear to me what was different about the two callsites.While I was in here I figured I would refactor things into a structure to make it easier to understand for my future self (and hopefully other readers)
What changes are included in this PR?
PullUpCorrelatedExpr::newand improve documentationAre these changes tested?
Covered by existing CI
Are there any user-facing changes?