You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "berkaysynnada (via GitHub)" <gi...@apache.org> on 2024/09/20 14:30:44 UTC
[PR] Feature/fix pushdown logic [datafusion]
berkaysynnada opened a new pull request, #12559:
URL: https://github.com/apache/datafusion/pull/12559
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example Closes #123 indicates that this PR will close issue #123.
-->
Closes #12558.
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
HashJoinExec and NestedLoopJoinExec maintains the order of right child. If a SortExec appears at the top of these joins, now it can be pushed down through its right child if all the requirements come from right child.
pushdown_requirement_to_children() logic was problematic for join operators. It is generalized now for any kind of operators if it falls to else{} block.
Nested brought a bug
HashJoin cannot pushdown
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
`pushdown_requirement_to_children()` function in `sort_pushdown.rs` is responsible for determining whether a sort operation can be pushed down over an operator, and if so, how the requirements are propagated. While most operators are handled properly, the mentioned operators (`HashJoinExec` and `NestedLoopJoinExec`) and other newly added operators may encounter issues due to missing index updates and the not utilizing the generic maintains flags. Now they are handled via `handle_custom_pushdown()`
## Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
Yes.
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the api change label.
-->
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix and Improve Sort Pushdown for Nested Loop and Hash Join [datafusion]
Posted by "austin362667 (via GitHub)" <gi...@apache.org>.
austin362667 commented on PR #12559:
URL: https://github.com/apache/datafusion/pull/12559#issuecomment-2365173975
Thank you @berkaysynnada The **J**oin **O**rdering **B**enchmark results turned out to be pretty promising against the PR!
Here are the results, comparing runs on IMDB between https://github.com/apache/datafusion/pull/12529 and this PR after rebasing.
#### FYR:
* [JOB issue](https://github.com/apache/datafusion/issues/12311)
* [JOB PR 1/N](https://github.com/apache/datafusion/pull/12497)
* [JOB PR 2/N](https://github.com/apache/datafusion/pull/12529)
```
austin@Machine ~/D/arrow-datafusion (imdb-benchmark-on-pr#12559)> ./benchmarks/bench.sh compare imdb-benchmark imdb-benchmark-on-pr#12559 (base)
Comparing imdb-benchmark and imdb-benchmark-on-pr#12559
--------------------
Benchmark imdb.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query ┃ imdb-benchmark ┃ imdb-benchmark-on-pr#12559 ┃ Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 1 │ 427.68ms │ 220.25ms │ +1.94x faster │
│ QQuery 2 │ 239.03ms │ 194.00ms │ +1.23x faster │
│ QQuery 3 │ 186.16ms │ 182.17ms │ no change │
│ QQuery 4 │ 229.37ms │ 212.17ms │ +1.08x faster │
│ QQuery 5 │ 223.12ms │ 210.09ms │ +1.06x faster │
│ QQuery 6 │ 224.09ms │ 206.91ms │ +1.08x faster │
│ QQuery 7 │ 218.43ms │ 196.12ms │ +1.11x faster │
│ QQuery 8 │ 312.08ms │ 246.01ms │ +1.27x faster │
│ QQuery 9 │ 918.78ms │ 712.36ms │ +1.29x faster │
│ QQuery 10 │ 796.41ms │ 665.34ms │ +1.20x faster │
│ QQuery 11 │ 935.68ms │ 770.40ms │ +1.21x faster │
│ QQuery 12 │ 268.38ms │ 221.41ms │ +1.21x faster │
│ QQuery 13 │ 223.23ms │ 182.92ms │ +1.22x faster │
│ QQuery 14 │ 313.61ms │ 243.80ms │ +1.29x faster │
│ QQuery 15 │ 966.59ms │ 787.50ms │ +1.23x faster │
│ QQuery 16 │ 985.37ms │ 711.68ms │ +1.38x faster │
│ QQuery 17 │ 919.12ms │ 808.42ms │ +1.14x faster │
│ QQuery 18 │ 2927.84ms │ 2512.75ms │ +1.17x faster │
│ QQuery 19 │ 2895.11ms │ 2609.02ms │ +1.11x faster │
│ QQuery 20 │ 2759.74ms │ 2510.42ms │ +1.10x faster │
│ QQuery 21 │ 3128.71ms │ 2617.79ms │ +1.20x faster │
│ QQuery 22 │ 3057.45ms │ 2699.52ms │ +1.13x faster │
│ QQuery 23 │ 3573.51ms │ 2966.13ms │ +1.20x faster │
│ QQuery 24 │ 920.38ms │ 746.90ms │ +1.23x faster │
│ QQuery 25 │ 833.97ms │ 691.30ms │ +1.21x faster │
│ QQuery 26 │ 1327.76ms │ 1104.20ms │ +1.20x faster │
│ QQuery 27 │ 790.45ms │ 739.20ms │ +1.07x faster │
│ QQuery 28 │ 858.53ms │ 774.95ms │ +1.11x faster │
│ QQuery 29 │ 7372.84ms │ 5554.50ms │ +1.33x faster │
│ QQuery 30 │ 7402.80ms │ 5297.99ms │ +1.40x faster │
│ QQuery 31 │ 1349.99ms │ 1185.69ms │ +1.14x faster │
│ QQuery 32 │ 1416.36ms │ 1064.89ms │ +1.33x faster │
│ QQuery 33 │ 1452.93ms │ 1258.74ms │ +1.15x faster │
│ QQuery 34 │ 1493.34ms │ 1336.39ms │ +1.12x faster │
│ QQuery 35 │ 1092.89ms │ 830.38ms │ +1.32x faster │
│ QQuery 36 │ 910.28ms │ 775.46ms │ +1.17x faster │
│ QQuery 37 │ 900.57ms │ 834.69ms │ +1.08x faster │
│ QQuery 38 │ 280.75ms │ 257.59ms │ +1.09x faster │
│ QQuery 39 │ 286.03ms │ 259.79ms │ +1.10x faster │
│ QQuery 40 │ 272.78ms │ 247.01ms │ +1.10x faster │
│ QQuery 41 │ 768.78ms │ 747.51ms │ no change │
│ QQuery 42 │ 844.27ms │ 823.68ms │ no change │
│ QQuery 43 │ 1428.58ms │ 1405.62ms │ no change │
│ QQuery 44 │ 902.23ms │ 860.41ms │ no change │
│ QQuery 45 │ 952.23ms │ 868.16ms │ +1.10x faster │
│ QQuery 46 │ 733.12ms │ 660.46ms │ +1.11x faster │
│ QQuery 47 │ 824.63ms │ 615.40ms │ +1.34x faster │
│ QQuery 48 │ 882.41ms │ 724.64ms │ +1.22x faster │
│ QQuery 49 │ 1008.04ms │ 844.56ms │ +1.19x faster │
│ QQuery 50 │ 962.39ms │ 887.13ms │ +1.08x faster │
│ QQuery 51 │ 999.78ms │ 873.18ms │ +1.14x faster │
│ QQuery 52 │ 1230.14ms │ 975.27ms │ +1.26x faster │
│ QQuery 53 │ 1072.94ms │ 1003.83ms │ +1.07x faster │
│ QQuery 54 │ 1227.30ms │ 1057.52ms │ +1.16x faster │
│ QQuery 55 │ 487.65ms │ 387.21ms │ +1.26x faster │
│ QQuery 56 │ 19721.02ms │ 15483.57ms │ +1.27x faster │
│ QQuery 57 │ 17101.19ms │ 15795.62ms │ +1.08x faster │
│ QQuery 58 │ 16990.06ms │ 15911.19ms │ +1.07x faster │
│ QQuery 59 │ 17443.75ms │ 15699.27ms │ +1.11x faster │
│ QQuery 60 │ 12064.59ms │ 8046.92ms │ +1.50x faster │
│ QQuery 61 │ 25606.02ms │ 23163.57ms │ +1.11x faster │
│ QQuery 62 │ 25240.80ms │ 23208.08ms │ +1.09x faster │
│ QQuery 63 │ 25238.73ms │ 26903.31ms │ 1.07x slower │
│ QQuery 64 │ 9405.30ms │ 9610.06ms │ no change │
│ QQuery 65 │ 28449.72ms │ 27333.06ms │ no change │
│ QQuery 66 │ 2910.74ms │ 2654.12ms │ +1.10x faster │
│ QQuery 67 │ 1835.37ms │ 1391.51ms │ +1.32x faster │
│ QQuery 68 │ 1864.57ms │ 1485.19ms │ +1.26x faster │
│ QQuery 69 │ 2362.05ms │ 1998.11ms │ +1.18x faster │
│ QQuery 70 │ 1833.31ms │ 1985.28ms │ 1.08x slower │
│ QQuery 71 │ 1946.16ms │ 2007.06ms │ no change │
│ QQuery 72 │ 2014.85ms │ 2226.71ms │ 1.11x slower │
│ QQuery 73 │ 985.92ms │ 1082.50ms │ 1.10x slower │
│ QQuery 74 │ 1227.04ms │ 1262.90ms │ no change │
│ QQuery 75 │ 1434.08ms │ 1131.45ms │ +1.27x faster │
│ QQuery 76 │ 895.58ms │ 933.90ms │ no change │
│ QQuery 77 │ 975.93ms │ 901.22ms │ +1.08x faster │
│ QQuery 78 │ 1163.51ms │ 976.54ms │ +1.19x faster │
│ QQuery 79 │ 1335.79ms │ 1120.72ms │ +1.19x faster │
│ QQuery 80 │ 1070.18ms │ 1142.63ms │ 1.07x slower │
│ QQuery 81 │ 1340.66ms │ 1252.60ms │ +1.07x faster │
│ QQuery 82 │ 1518.97ms │ 1419.51ms │ +1.07x faster │
│ QQuery 83 │ 1905.73ms │ 1175.87ms │ +1.62x faster │
│ QQuery 84 │ 1515.77ms │ 1040.71ms │ +1.46x faster │
│ QQuery 85 │ 1151.02ms │ 1179.61ms │ no change │
│ QQuery 86 │ 2680.10ms │ 2436.08ms │ +1.10x faster │
│ QQuery 87 │ 2209.13ms │ 2170.94ms │ no change │
│ QQuery 88 │ 1494.10ms │ 1542.68ms │ no change │
│ QQuery 89 │ 1453.42ms │ 1551.83ms │ 1.07x slower │
│ QQuery 90 │ 1976.67ms │ 1619.60ms │ +1.22x faster │
│ QQuery 91 │ 1091.85ms │ 1094.07ms │ no change │
│ QQuery 92 │ 1194.71ms │ 968.17ms │ +1.23x faster │
│ QQuery 93 │ 1064.62ms │ 1051.08ms │ no change │
│ QQuery 94 │ 1040.22ms │ 1001.40ms │ no change │
│ QQuery 95 │ 1045.82ms │ 915.07ms │ +1.14x faster │
│ QQuery 96 │ 1213.03ms │ 992.77ms │ +1.22x faster │
│ QQuery 97 │ 1078.71ms │ 1011.11ms │ +1.07x faster │
│ QQuery 98 │ 1071.27ms │ 911.23ms │ +1.18x faster │
│ QQuery 99 │ 1159.91ms │ 1003.24ms │ +1.16x faster │
│ QQuery 100 │ 1845.79ms │ 1748.37ms │ +1.06x faster │
│ QQuery 101 │ 1804.21ms │ 1589.81ms │ +1.13x faster │
│ QQuery 102 │ 2440.43ms │ 2105.23ms │ +1.16x faster │
│ QQuery 103 │ 1564.05ms │ 1236.40ms │ +1.27x faster │
│ QQuery 104 │ 1410.36ms │ 1276.40ms │ +1.10x faster │
│ QQuery 105 │ 1765.29ms │ 1302.86ms │ +1.35x faster │
│ QQuery 106 │ 1862.78ms │ 1452.58ms │ +1.28x faster │
│ QQuery 107 │ 1962.28ms │ 1352.52ms │ +1.45x faster │
│ QQuery 108 │ 1685.42ms │ 1488.34ms │ +1.13x faster │
│ QQuery 109 │ 348.77ms │ 237.42ms │ +1.47x faster │
│ QQuery 110 │ 380.69ms │ 245.74ms │ +1.55x faster │
│ QQuery 111 │ 555.95ms │ 461.32ms │ +1.21x faster │
│ QQuery 112 │ 409.35ms │ 408.67ms │ no change │
│ QQuery 113 │ 553.14ms │ 483.32ms │ +1.14x faster │
└──────────────┴────────────────┴────────────────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┓
┃ Benchmark Summary ┃ ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━┩
│ Total Time (imdb-benchmark) │ 336927.11ms │
│ Total Time (imdb-benchmark-on-pr#12559) │ 301562.45ms │
│ Average Time (imdb-benchmark) │ 2981.66ms │
│ Average Time (imdb-benchmark-on-pr#12559) │ 2668.69ms │
│ Queries Faster │ 90 │
│ Queries Slower │ 6 │
│ Queries with No Change │ 17 │
└───────────────────────────────────────────┴─────────────┘
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix and Improve Sort Pushdown for Nested Loop and Hash Join [datafusion]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #12559:
URL: https://github.com/apache/datafusion/pull/12559#issuecomment-2368258117
TY @berkaysynnada
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix and Improve Sort Pushdown for Nested Loop and Hash Join [datafusion]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan merged PR #12559:
URL: https://github.com/apache/datafusion/pull/12559
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix and Improve Sort Pushdown for Nested Loop and Hash Join [datafusion]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #12559:
URL: https://github.com/apache/datafusion/pull/12559#issuecomment-2365175831
Nice, impressive results
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org