Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LimitPushdown rule uncorrect remove some GlobalLimitExec #14204

Closed
Tracked by #14008
haohuaijin opened this issue Jan 20, 2025 · 4 comments · Fixed by #14245
Closed
Tracked by #14008

LimitPushdown rule uncorrect remove some GlobalLimitExec #14204

haohuaijin opened this issue Jan 20, 2025 · 4 comments · Fixed by #14245
Assignees
Labels
bug Something isn't working

Comments

@haohuaijin
Copy link
Contributor

Describe the bug

see the plan

| physical_plan after OutputRequirements                     | GlobalLimitExec: skip=0, fetch=10                                                                                                                                                                              |
|                                                            |   ProjectionExec: expr=[a@2 as a, b@3 as b, a@0 as a, b@1 as b]                                                                                                                                                |
|                                                            |     CrossJoinExec                                                                                                                                                                                              |
|                                                            |       GlobalLimitExec: skip=0, fetch=1                                                                                                                                                                         |
|                                                            |         MemoryExec: partitions=1, partition_sizes=[1]                                                                                                                                                          |
|                                                            |       GlobalLimitExec: skip=0, fetch=10                                                                                                                                                                        |
|                                                            |         MemoryExec: partitions=1, partition_sizes=[1]                                                                                                                                                          |
|                                                            |                                                                                                                                                                                                                |
| physical_plan after LimitAggregation                       | SAME TEXT AS ABOVE                                                                                                                                                                                             |
| physical_plan after ProjectionPushdown                     | SAME TEXT AS ABOVE                                                                                                                                                                                             |
| physical_plan after LimitPushdown                          | ProjectionExec: expr=[a@2 as a, b@3 as b, a@0 as a, b@1 as b]                                                                                                                                                  |
|                                                            |   GlobalLimitExec: skip=0, fetch=10                                                                                                                                                                            |
|                                                            |     CrossJoinExec                                                                                                                                                                                              |
|                                                            |       MemoryExec: partitions=1, partition_sizes=[1]                                                                                                                                                            |
|                                                            |       MemoryExec: partitions=1, partition_sizes=[1]  

To Reproduce

> create table t(a int, b int) as values (1,2), (2,3), (3,4);
0 row(s) fetched. 
Elapsed 0.036 seconds.

> select * from t as t1 join (select * from t limit 1) limit 10;
+---+---+---+---+
| a | b | a | b |
+---+---+---+---+
| 1 | 2 | 1 | 2 |
| 2 | 3 | 1 | 2 |
| 3 | 4 | 1 | 2 |
| 1 | 2 | 2 | 3 |
| 2 | 3 | 2 | 3 |
| 3 | 4 | 2 | 3 |
| 1 | 2 | 3 | 4 |
| 2 | 3 | 3 | 4 |
| 3 | 4 | 3 | 4 |
+---+---+---+---+
9 row(s) fetched. 
Elapsed 0.013 seconds.

Expected behavior

check in datafusion v43, v44 and main, both have this problem

Additional context

if add a order by, the result will correct

> create table t(a int, b int) as values (1,2), (2,3), (3,4);
0 row(s) fetched. 
Elapsed 0.036 seconds.

> select * from t as t1 join (select * from t limit 1) limit 10;
+---+---+---+---+
| a | b | a | b |
+---+---+---+---+
| 1 | 2 | 1 | 2 |
| 2 | 3 | 1 | 2 |
| 3 | 4 | 1 | 2 |
| 1 | 2 | 2 | 3 |
| 2 | 3 | 2 | 3 |
| 3 | 4 | 2 | 3 |
| 1 | 2 | 3 | 4 |
| 2 | 3 | 3 | 4 |
| 3 | 4 | 3 | 4 |
+---+---+---+---+
9 row(s) fetched. 
Elapsed 0.013 seconds.

> select * from t as t1 join (select * from t order by a limit 1) limit 10;
+---+---+---+---+
| a | b | a | b |
+---+---+---+---+
| 1 | 2 | 1 | 2 |
| 2 | 3 | 1 | 2 |
| 3 | 4 | 1 | 2 |
+---+---+---+---+
3 row(s) fetched. 
Elapsed 0.014 seconds.
@jonahgao
Copy link
Member

Related to #12003

@berkaysynnada
Copy link
Contributor

The fix might be similar to #14192

@zhuqi-lucas
Copy link
Contributor

take

@zhuqi-lucas
Copy link
Contributor

Submitted a PR for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants