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

Fix and Improve Sort Pushdown for Nested Loop and Hash Join #12559

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

berkaysynnada
Copy link
Contributor

@berkaysynnada berkaysynnada commented Sep 20, 2024

Which issue does this PR close?

Closes #12558.

Rationale for this change

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 the 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.

What changes are included 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 not utilizing the generic maintains flags. Now they are handled via handle_custom_pushdown().

Are these changes tested?

Yes

Are there any user-facing changes?

@github-actions github-actions bot added core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Sep 20, 2024
@berkaysynnada berkaysynnada changed the title Feature/fix pushdown logic Fix and Improve Sort Pushdown for Nested Loop and Hash Join Sep 20, 2024
@austin362667
Copy link
Contributor

austin362667 commented Sep 21, 2024

Thank you @berkaysynnada The Join Ordering Benchmark results turned out to be pretty promising against your PR!
Here are the results, comparing runs on IMDB between #12529 and this PR after rebasing.

FYR:

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 │
└───────────────────────────────────────────┴─────────────┘

@Dandandan
Copy link
Contributor

Nice, impressive results

Copy link
Contributor

@ozankabak ozankabak left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@Dandandan Dandandan merged commit b00723a into apache:main Sep 23, 2024
29 of 30 checks passed
@Dandandan
Copy link
Contributor

TY @berkaysynnada

bgjackma pushed a commit to bgjackma/datafusion that referenced this pull request Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sort Pushdown Causes an Invalid Plan
4 participants