Skip to content

Conversation

@jackwener
Copy link
Member

@jackwener jackwener commented Nov 13, 2022

Which issue does this PR close?

Closes #3864.

We can propagate empty_relation to eliminate useless plan.

This is a bottom-up rule.

| logical_plan after eliminate_filter| Projection: t1.column1                        |
|                                    |   Inner Join: t1.column1 = ta1.column1        |
|                                    |     TableScan: t1                             |
|                                    |     Projection: ta1.column1, alias=ta1        |
|                                    |       Projection: t2.column1, alias=ta1       |
|                                    |         Inner Join: t2.column1 = ta2.column1  |
|                                    |           TableScan: t2                       |
|                                    |           Projection: ta2.column1, alias=ta2  |
|                                    |             Projection: t3.column1, alias=ta2 |
|                                    |               EmptyRelation                   |
| logical_plan after eliminate_limit | EmptyRelation                                 |

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added logical-expr Logical plan and expressions optimizer Optimizer rules labels Nov 13, 2022
@jackwener jackwener changed the title add propagate_empty_relation add propagate_empty_relation optimizer rule Nov 13, 2022
@github-actions github-actions bot removed the logical-expr Logical plan and expressions label Nov 13, 2022
@github-actions github-actions bot added the logical-expr Logical plan and expressions label Nov 14, 2022
@jackwener
Copy link
Member Author

Thanks @mingmwang.
Condition for Join is complex😂, I leave a TODO for it.

@mingmwang
Copy link
Contributor

Thank you so much @jackwener. Overall, the PR LGTM.

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing I would recommend it at least one test in https://github.com/apache/arrow-datafusion/blob/master/datafusion/optimizer/tests/integration-test.rs that shows this optimization working with other optimization passes

@alamb
Copy link
Contributor

alamb commented Nov 14, 2022

Thank you @jackwener

@jackwener
Copy link
Member Author

Thanks @mingmwang @alamb @Dandandan review❤️

@jackwener jackwener mentioned this pull request Nov 15, 2022
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really nice @jackwener -- thank you so much.

I'll wait a day to see if @mingmwang or @Dandandan want to review again as well and then I'll plan to merge it in.

fn propagate_empty_relation() {
let sql = "SELECT col_int32 FROM test JOIN ( SELECT col_int32 FROM test WHERE false ) AS ta1 ON test.col_int32 = ta1.col_int32;";
let plan = test_sql(sql).unwrap();
// when children exist EmptyRelation, it will bottom-up propagate.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@alamb alamb merged commit 5de9709 into apache:master Nov 16, 2022
@alamb
Copy link
Contributor

alamb commented Nov 16, 2022

Thanks again @jackwener

@ursabot
Copy link

ursabot commented Nov 16, 2022

Benchmark runs are scheduled for baseline = f0359a7 and contender = 5de9709. 5de9709 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

logical-expr Logical plan and expressions optimizer Optimizer rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace Filter: Boolean(false) with EmptyRelation

5 participants