feat: Decorrelate more predicate subqueries #12945

eejbyfeldt · 2024-10-15T18:45:55Z

Which issue does this PR close?

Closes #5368.

Rationale for this change

Support subqueries in more cases and needed for TPC-DS (#4763)

What changes are included in this PR?

Extends the DecorrelatePredicateSubquery optimizer rule to not only handle predicate subquries in the "top" of a conjunctive filter, but also in other expressions like OR. This is done by replacing the subquery by an existence join and replacing the subquery expression with the exists column. Since datafusion does not natively support existence join/mark join (yet) we emulate it by adding a boolean column and checking for null after the join.

Are these changes tested?

More tests in subquery.slt (also move some of the existing spec that needed updating with these changes).

Are there any user-facing changes?

Yes, support for predicate subqueries in more cases.

eejbyfeldt · 2024-10-16T12:10:59Z

datafusion/optimizer/src/decorrelate_predicate_subquery.rs

@@ -1407,13 +1361,13 @@ mod tests {
            .build()?;

        let expected = "Projection: customer.c_custkey [c_custkey:Int64]\
-                        \n  LeftSemi Join:  Filter: __correlated_sq_1.o_custkey = customer.c_custkey [c_custkey:Int64, c_name:Utf8]\


This change is because I fixed how the rule first rewrites nested subqueries first. The old logic did not traverse the nested subquery but instead just tried to rewrite the first node, which did not really work since it would be a Subquery node not a filter.

The changes are that the auto generated subquery alias (e.g. __correlated_sq_1) is a different order, right? Or are you referring to something more substantial?

No, only the order the alias is assigned.

comphead

Thanks @eejbyfeldt
Wondering if there is any performance improvement/degradation after the decorrelation

alamb

Thank you @eejbyfeldt -- this is looking pretty exciting.

I have some testing questions but overall the code is looking quite nice. Thank you (as always)

Sadly, I double checked and this does not appear to fix #5265

alamb · 2024-10-18T20:36:16Z

datafusion/optimizer/src/decorrelate_predicate_subquery.rs

@@ -442,60 +520,6 @@ mod tests {
        assert_optimized_plan_equal(plan, expected)
    }

-    /// Test for IN subquery with additional OR filter


Why are these tests removed?

They were there to verify that the previous code did not handle these kind of subqueries, so their expected output needed to be updated. But the new sqllogictests provide the same coverage, so opted for removing these instead of updating them.

alamb · 2024-10-18T20:37:03Z

datafusion/optimizer/src/decorrelate_predicate_subquery.rs

@@ -1407,13 +1361,13 @@ mod tests {
            .build()?;

        let expected = "Projection: customer.c_custkey [c_custkey:Int64]\
-                        \n  LeftSemi Join:  Filter: __correlated_sq_1.o_custkey = customer.c_custkey [c_custkey:Int64, c_name:Utf8]\


The changes are that the auto generated subquery alias (e.g. __correlated_sq_1) is a different order, right? Or are you referring to something more substantial?

alamb · 2024-10-18T20:37:40Z

datafusion/sqllogictest/test_files/subquery.slt

@@ -1028,6 +1028,80 @@ false
 true
 true

+# in_subquery_to_join_with_correlated_outer_filter_disjunction


Could you also add EXPLAIN tests for these queries too so we can review if the plans will generate the correct results?

Added in ea5d04e

eejbyfeldt · 2024-10-19T17:54:06Z

Wondering if there is any performance improvement/degradation after the decorrelation

This should make us able to run quries where we previously failed with

This feature is not implemented: Physical plan does not support logical expression Exists(Exists { subquery: <subquery>, negated: false })

so the is no "defined" performance change.

comphead

I think it is lgtm, thanks @eejbyfeldt

alamb · 2024-10-20T12:48:03Z

Thanks again @eejbyfeldt

In apache#12945 the emulation of an mark join has a bug when there is duplicate values in the subquery. This would be fixable by adding a distinct before the join. But this patch instead implements a LeftMark join with the desired semantics and uses that. The LeftMark join will return a row for each in the left input with an additional column "mark" that is true if there was a match in the right input and false otherwise. Note: This patch does not implement the full null semantics for the mark join described in http://btw2017.informatik.uni-stuttgart.de/slidesandpapers/F1-10-37/paper_web.pdf which which will be needed if we and `ANY` subqueries. The version is this patch the mark column will only be true for had a match and false when no match was found, never `null`.

* Implement LeftMark join In #12945 the emulation of an mark join has a bug when there is duplicate values in the subquery. This would be fixable by adding a distinct before the join. But this patch instead implements a LeftMark join with the desired semantics and uses that. The LeftMark join will return a row for each in the left input with an additional column "mark" that is true if there was a match in the right input and false otherwise. Note: This patch does not implement the full null semantics for the mark join described in http://btw2017.informatik.uni-stuttgart.de/slidesandpapers/F1-10-37/paper_web.pdf which which will be needed if we and `ANY` subqueries. The version is this patch the mark column will only be true for had a match and false when no match was found, never `null`. * Use mark join in decorrelate subqueries This fixes a correctness issue in the current approach. * Add physical plan sqllogictest * fmt * Fix join type in doc comment * Minor clean ups * Add more documentation to LeftMark join * Remove qualification * fix doc --------- Co-authored-by: Andrew Lamb <[email protected]>

github-actions bot added optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Oct 15, 2024

eejbyfeldt force-pushed the i-5368-predicate-subquery-complex-expression branch 2 times, most recently from 939e2b4 to 5050a4e Compare October 16, 2024 11:35

github-actions bot added the substrait label Oct 16, 2024

eejbyfeldt force-pushed the i-5368-predicate-subquery-complex-expression branch from 5050a4e to 81cd07d Compare October 16, 2024 11:48

eejbyfeldt commented Oct 16, 2024

View reviewed changes

eejbyfeldt force-pushed the i-5368-predicate-subquery-complex-expression branch from 81cd07d to 0a13d91 Compare October 16, 2024 12:11

eejbyfeldt marked this pull request as ready for review October 16, 2024 12:35

comphead reviewed Oct 17, 2024

View reviewed changes

alamb reviewed Oct 18, 2024

View reviewed changes

eejbyfeldt added 2 commits October 19, 2024 19:39

Decorrelate more predicate subqueries

fd9d471

Added sqllogictest explain tests

ea5d04e

eejbyfeldt force-pushed the i-5368-predicate-subquery-complex-expression branch from 0a13d91 to ea5d04e Compare October 19, 2024 17:53

eejbyfeldt requested review from comphead and alamb October 19, 2024 18:06

comphead approved these changes Oct 19, 2024

View reviewed changes

eejbyfeldt changed the title ~~Decorrelate more predicate subqueries~~ feat: Decorrelate more predicate subqueries Oct 20, 2024

alamb merged commit 972e3ab into apache:main Oct 20, 2024
25 checks passed

alamb mentioned this pull request Oct 21, 2024

Oct 21, 2024: This week in DataFusion #13035

Closed

4 tasks

eejbyfeldt mentioned this pull request Oct 27, 2024

feat: Implement LeftMark join to fix subquery correctness issue #13134

Merged

eejbyfeldt mentioned this pull request Oct 27, 2024

Decorrelated predicate subqueries with disjuncition has duplicated rows #13135

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Decorrelate more predicate subqueries #12945

feat: Decorrelate more predicate subqueries #12945

eejbyfeldt commented Oct 15, 2024 •

edited

Loading

eejbyfeldt Oct 16, 2024

alamb Oct 18, 2024

eejbyfeldt Oct 19, 2024

comphead left a comment

alamb left a comment

alamb Oct 18, 2024

eejbyfeldt Oct 19, 2024

alamb Oct 18, 2024

alamb Oct 18, 2024

eejbyfeldt Oct 19, 2024

eejbyfeldt commented Oct 19, 2024

comphead left a comment

alamb commented Oct 20, 2024

feat: Decorrelate more predicate subqueries #12945

feat: Decorrelate more predicate subqueries #12945

Conversation

eejbyfeldt commented Oct 15, 2024 • edited Loading

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

comphead left a comment

Choose a reason for hiding this comment

alamb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eejbyfeldt commented Oct 19, 2024

comphead left a comment

Choose a reason for hiding this comment

alamb commented Oct 20, 2024

eejbyfeldt commented Oct 15, 2024 •

edited

Loading