You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "ctsk (via GitHub)" <gi...@apache.org> on 2025/04/20 16:36:56 UTC
[PR] Set HashJoin seed [datafusion]
ctsk opened a new pull request, #15783:
URL: https://github.com/apache/datafusion/pull/15783
## Which issue does this PR close?
- Closes #15620.
## What changes are included in this PR?
The hash join seed is hard-coded to a different value that the RepartitionExec seed.
## Are these changes tested?
Covered by existing tests.
## Are there any user-facing changes?
No.
--
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] Set HashJoin seed [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #15783:
URL: https://github.com/apache/datafusion/pull/15783#issuecomment-2838662506
Thanks again @ctsk -- sorry for the delay in review / merge
--
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] Set HashJoin seed [datafusion]
Posted by "Weijun-H (via GitHub)" <gi...@apache.org>.
Weijun-H commented on code in PR #15783:
URL: https://github.com/apache/datafusion/pull/15783#discussion_r2052164212
##########
datafusion/physical-plan/src/joins/hash_join.rs:
##########
@@ -86,6 +86,9 @@ use datafusion_physical_expr_common::physical_expr::fmt_sql;
use futures::{ready, Stream, StreamExt, TryStreamExt};
use parking_lot::Mutex;
+const HASH_JOIN_SEED: RandomState =
Review Comment:
maybe left some comments here to explain this change
```suggestion
/// Hard-coded seed to ensure hash values from the hash join differ from `RepartitionExec`, avoiding collisions.
const HASH_JOIN_SEED: RandomState =
```
--
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] Set HashJoin seed [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #15783:
URL: https://github.com/apache/datafusion/pull/15783#discussion_r2065171783
##########
datafusion/physical-plan/src/joins/hash_join.rs:
##########
@@ -86,6 +86,10 @@ use datafusion_physical_expr_common::physical_expr::fmt_sql;
use futures::{ready, Stream, StreamExt, TryStreamExt};
use parking_lot::Mutex;
+/// Hard-coded seed to ensure hash values from the hash join differ from `RepartitionExec`, avoiding collisions.
+const HASH_JOIN_SEED: RandomState =
+ RandomState::with_seeds('J' as u64, 'O' as u64, 'I' as u64, 'N' as u64);
Review Comment:
Love it
--
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] Set HashJoin seed [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #15783:
URL: https://github.com/apache/datafusion/pull/15783#issuecomment-2837217773
๐ค: Benchmark completed
<details><summary>Details</summary>
<p>
```
Comparing HEAD and fix_hash-join-seed
--------------------
Benchmark clickbench_extended.json
--------------------
โโโโโโโโโโโโโโโโณโโโโโโโโโโโโโณโโโโโโโโโโโโโโโโโโโโโณโโโโโโโโโโโโ
โ Query โ HEAD โ fix_hash-join-seed โ Change โ
โกโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโฉ
โ QQuery 0 โ 2000.85ms โ 1972.89ms โ no change โ
โ QQuery 1 โ 714.99ms โ 712.92ms โ no change โ
โ QQuery 2 โ 1487.22ms โ 1481.43ms โ no change โ
โ QQuery 3 โ 718.29ms โ 698.86ms โ no change โ
โ QQuery 4 โ 1458.20ms โ 1457.44ms โ no change โ
โ QQuery 5 โ 15666.90ms โ 15335.98ms โ no change โ
โ QQuery 6 โ 2082.26ms โ 2092.29ms โ no change โ
โโโโโโโโโโโโโโโโดโโโโโโโโโโโโโดโโโโโโโโโโโโโโโโโโโโโดโโโโโโโโโโโโ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโณโโโโโโโโโโโโโ
โ Benchmark Summary โ โ
โกโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโฉ
โ Total Time (HEAD) โ 24128.70ms โ
โ Total Time (fix_hash-join-seed) โ 23751.80ms โ
โ Average Time (HEAD) โ 3446.96ms โ
โ Average Time (fix_hash-join-seed) โ 3393.11ms โ
โ Queries Faster โ 0 โ
โ Queries Slower โ 0 โ
โ Queries with No Change โ 7 โ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโดโโโโโโโโโโโโโ
--------------------
Benchmark clickbench_partitioned.json
--------------------
โโโโโโโโโโโโโโโโณโโโโโโโโโโโโโณโโโโโโโโโโโโโโโโโโโโโณโโโโโโโโโโโโโโโโ
โ Query โ HEAD โ fix_hash-join-seed โ Change โ
โกโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโฉ
โ QQuery 0 โ 2.17ms โ 2.22ms โ no change โ
โ QQuery 1 โ 37.42ms โ 37.09ms โ no change โ
โ QQuery 2 โ 92.65ms โ 90.26ms โ no change โ
โ QQuery 3 โ 103.22ms โ 97.99ms โ +1.05x faster โ
โ QQuery 4 โ 887.43ms โ 740.13ms โ +1.20x faster โ
โ QQuery 5 โ 897.35ms โ 867.53ms โ no change โ
โ QQuery 6 โ 2.43ms โ 2.32ms โ no change โ
โ QQuery 7 โ 44.13ms โ 42.08ms โ no change โ
โ QQuery 8 โ 945.09ms โ 901.37ms โ no change โ
โ QQuery 9 โ 1249.21ms โ 1241.44ms โ no change โ
โ QQuery 10 โ 279.17ms โ 274.20ms โ no change โ
โ QQuery 11 โ 315.83ms โ 312.81ms โ no change โ
โ QQuery 12 โ 957.03ms โ 916.08ms โ no change โ
โ QQuery 13 โ 1374.90ms โ 1355.30ms โ no change โ
โ QQuery 14 โ 876.51ms โ 853.30ms โ no change โ
โ QQuery 15 โ 1056.31ms โ 1033.93ms โ no change โ
โ QQuery 16 โ 1801.98ms โ 1744.01ms โ no change โ
โ QQuery 17 โ 1645.60ms โ 1614.23ms โ no change โ
โ QQuery 18 โ 3144.80ms โ 3093.86ms โ no change โ
โ QQuery 19 โ 86.32ms โ 85.42ms โ no change โ
โ QQuery 20 โ 1161.10ms โ 1155.16ms โ no change โ
โ QQuery 21 โ 1363.22ms โ 1365.95ms โ no change โ
โ QQuery 22 โ 2284.72ms โ 2297.40ms โ no change โ
โ QQuery 23 โ 8516.40ms โ 8523.32ms โ no change โ
โ QQuery 24 โ 487.74ms โ 486.22ms โ no change โ
โ QQuery 25 โ 412.34ms โ 408.17ms โ no change โ
โ QQuery 26 โ 543.10ms โ 550.15ms โ no change โ
โ QQuery 27 โ 1702.50ms โ 1729.68ms โ no change โ
โ QQuery 28 โ 12489.17ms โ 12636.66ms โ no change โ
โ QQuery 29 โ 541.01ms โ 535.21ms โ no change โ
โ QQuery 30 โ 829.92ms โ 821.21ms โ no change โ
โ QQuery 31 โ 870.53ms โ 883.44ms โ no change โ
โ QQuery 32 โ 2658.55ms โ 2675.61ms โ no change โ
โ QQuery 33 โ 3386.29ms โ 3381.19ms โ no change โ
โ QQuery 34 โ 3385.05ms โ 3376.41ms โ no change โ
โ QQuery 35 โ 1283.08ms โ 1258.65ms โ no change โ
โ QQuery 36 โ 129.46ms โ 132.73ms โ no change โ
โ QQuery 37 โ 59.66ms โ 58.32ms โ no change โ
โ QQuery 38 โ 127.07ms โ 126.98ms โ no change โ
โ QQuery 39 โ 206.89ms โ 202.78ms โ no change โ
โ QQuery 40 โ 52.69ms โ 50.45ms โ no change โ
โ QQuery 41 โ 45.41ms โ 47.94ms โ 1.06x slower โ
โ QQuery 42 โ 40.69ms โ 40.16ms โ no change โ
โโโโโโโโโโโโโโโโดโโโโโโโโโโโโโดโโโโโโโโโโโโโโโโโโโโโดโโโโโโโโโโโโโโโโ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโณโโโโโโโโโโโโโ
โ Benchmark Summary โ โ
โกโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโฉ
โ Total Time (HEAD) โ 58376.12ms โ
โ Total Time (fix_hash-join-seed) โ 58049.37ms โ
โ Average Time (HEAD) โ 1357.58ms โ
โ Average Time (fix_hash-join-seed) โ 1349.99ms โ
โ Queries Faster โ 2 โ
โ Queries Slower โ 1 โ
โ Queries with No Change โ 40 โ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโดโโโโโโโโโโโโโ
--------------------
Benchmark tpch_mem_sf1.json
--------------------
โโโโโโโโโโโโโโโโณโโโโโโโโโโโณโโโโโโโโโโโโโโโโโโโโโณโโโโโโโโโโโโ
โ Query โ HEAD โ fix_hash-join-seed โ Change โ
โกโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโฉ
โ QQuery 1 โ 122.29ms โ 124.02ms โ no change โ
โ QQuery 2 โ 24.38ms โ 24.21ms โ no change โ
โ QQuery 3 โ 34.54ms โ 35.67ms โ no change โ
โ QQuery 4 โ 20.92ms โ 20.74ms โ no change โ
โ QQuery 5 โ 56.17ms โ 56.45ms โ no change โ
โ QQuery 6 โ 12.35ms โ 12.35ms โ no change โ
โ QQuery 7 โ 104.86ms โ 104.53ms โ no change โ
โ QQuery 8 โ 26.34ms โ 26.19ms โ no change โ
โ QQuery 9 โ 61.22ms โ 63.62ms โ no change โ
โ QQuery 10 โ 56.72ms โ 57.69ms โ no change โ
โ QQuery 11 โ 13.01ms โ 13.17ms โ no change โ
โ QQuery 12 โ 45.48ms โ 46.51ms โ no change โ
โ QQuery 13 โ 29.20ms โ 30.13ms โ no change โ
โ QQuery 14 โ 10.25ms โ 9.85ms โ no change โ
โ QQuery 15 โ 25.11ms โ 25.33ms โ no change โ
โ QQuery 16 โ 23.20ms โ 23.88ms โ no change โ
โ QQuery 17 โ 95.84ms โ 99.73ms โ no change โ
โ QQuery 18 โ 231.30ms โ 239.99ms โ no change โ
โ QQuery 19 โ 26.85ms โ 27.91ms โ no change โ
โ QQuery 20 โ 40.82ms โ 39.10ms โ no change โ
โ QQuery 21 โ 166.73ms โ 172.85ms โ no change โ
โ QQuery 22 โ 16.81ms โ 17.12ms โ no change โ
โโโโโโโโโโโโโโโโดโโโโโโโโโโโดโโโโโโโโโโโโโโโโโโโโโดโโโโโโโโโโโโ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโณโโโโโโโโโโโโ
โ Benchmark Summary โ โ
โกโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโฉ
โ Total Time (HEAD) โ 1244.41ms โ
โ Total Time (fix_hash-join-seed) โ 1271.05ms โ
โ Average Time (HEAD) โ 56.56ms โ
โ Average Time (fix_hash-join-seed) โ 57.77ms โ
โ Queries Faster โ 0 โ
โ Queries Slower โ 0 โ
โ Queries with No Change โ 22 โ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโดโโโโโโโโโโโโ
```
</p>
</details>
--
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] Set HashJoin seed [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #15783:
URL: https://github.com/apache/datafusion/pull/15783
--
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] Set HashJoin seed [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #15783:
URL: https://github.com/apache/datafusion/pull/15783#issuecomment-2837274656
> โ QQuery 4 โ 887.43ms โ 740.13ms โ +1.20x faster โ
Given these queries don't have joins I am not sure that is reproduceable ๐ฌ
--
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] Set HashJoin seed [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #15783:
URL: https://github.com/apache/datafusion/pull/15783#issuecomment-2837160708
๐ค `./gh_compare_branch.sh` [Benchmark Script](https://github.com/alamb/datafusion-benchmarking/blob/main/gh_compare_branch.sh) Running
Linux aal-dev 6.11.0-1013-gcp #13~24.04.1-Ubuntu SMP Wed Apr 2 16:34:16 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing fix/hash-join-seed (e1fc96def1b400a738e036b1415a6c0deff8e345) to 8a193c22f6c3f771bff16cb8f9608d764d268c59 [diff](https://github.com/apache/datafusion/compare/8a193c22f6c3f771bff16cb8f9608d764d268c59..e1fc96def1b400a738e036b1415a6c0deff8e345)
Benchmarks: tpch_mem clickbench_partitioned clickbench_extended
Results will be posted here when complete
--
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