Skip to content

Commit 1fa378d

Browse files
authored
perf: keep the hash join unique-key fast path when the build side has NULL keys (#26018)
## Which issue does this PR close? - None. There is no separate issue; the problem is described below. ## Rationale for this change When every build key is unique, the hash join probe uses a faster loop that skips walking the duplicate-key chain. That loop was enabled by `map.len() == next.len()`, which means "every build row is in the map". Under `NullEqualsNothing`, build rows with a NULL key are not inserted, because they can never match. So a single NULL build key turns the fast path off, and every probe walks the chain even though every chain has length 1. This affects the hash-map path: string keys, multi-column keys and wide-range integer keys. It does not affect `ArrayMap`. A nullable unique key on the build side is enough to trigger it. ## What changes are included in this PR? - `update_from_iter` returns whether it chained a row onto an existing hash value. `JoinHashMapU32` and `JoinHashMapU64` keep that result in a `chained` flag across batches. - `get_matched_indices_with_limit_offset` now takes the uniqueness decision as a parameter instead of computing `map.len() == next_chain.len()` itself. The two join hash maps pass `!chained`. `PruningJoinHashMap` (symmetric hash join) still passes `map.len() == next.len()`, so its behavior is unchanged. - Nothing changes when no build rows are skipped, because `map.len() == next.len()` held exactly when no row was chained. `NullEqualsNull` is also unchanged: NULL rows are inserted and chain as before. ## What is the testing strategy for this PR? New unit tests: - `join_hash_map::tests::test_unique_fast_path_with_uninserted_row`: unique keys plus one row that was not inserted now take the fast path. The test checks the fast path's offset behavior with `limit = 2`; before this change it failed with `Some((1, Some(0)))`. - `join_hash_map::tests::test_chain_from_earlier_update_disables_fast_path`: a chain created by an earlier batch keeps the chain walk. - `utils::tests::update_hash_null_key_does_not_change_matches`: `update_hash` plus a chunked probe give the same matches with and without a NULL build key. Benchmark: `datafusion-cli` built with `release-nonlto`, base vs. this PR, on an Apple M4 Pro (12 cores, 24 GB) with default `target_partitions`. Each query is `SELECT count(*) FROM probe p JOIN build b ON <keys>`. The build side has 5M unique keys (`Utf8View`, or `(Int64 wide range, Utf8View)`) plus one NULL-key row. The probe side has 20M rows in shuffled order and matches every key. Runs were interleaved; each value is the median of 18 runs (6 rounds × 3), started only when the 1-minute load average was below 4. Under default thresholds the 5M-row build side runs in `Partitioned` mode. The `CollectLeft` rows raise `hash_join_single_partition_threshold{,_rows}` so that the whole build side is one map. `EXPLAIN ANALYZE` shows `array_map_created_count=0` in every case except where noted. | Case | Mode | base (s) | PR (s) | PR/base | |---|---|---|---|---| | Utf8View key, unique + 1 NULL | CollectLeft | 0.237 | 0.214 | **0.90** | | (Int64, Utf8View) key, unique + 1 NULL | CollectLeft | 0.284 | 0.259 | **0.91** | | Utf8View key, unique + 1 NULL | Partitioned | 0.144 | 0.143 | 0.99 | | (Int64, Utf8View) key, unique + 1 NULL | Partitioned | 0.192 | 0.189 | 0.99 | | Control: Utf8View key, unique, no NULL | Partitioned | 0.134 | 0.137 | 1.02 | | Control: Utf8View key, unique, no NULL | CollectLeft | 0.205 | 0.210 | 1.03 | | Control: (Int64, Utf8View) key, unique, no NULL | CollectLeft | 0.253 | 0.253 | 1.00 | | Control: 2 rows per key + 1 NULL | Partitioned | 0.163 | 0.161 | 0.99 | | Control: 2 rows per key + 1 NULL | CollectLeft | 0.260 | 0.264 | 1.02 | | Control: `hj` Q13 (TPC-H SF10, 1% density, hash map), `count(*)` | CollectLeft | 0.067 | 0.068 | 1.02 | `HashJoinExec` `join_time` from `EXPLAIN ANALYZE` (summed over partitions, median of 7 runs): | Case | base | PR | |---|---|---| | Utf8View key + 1 NULL, CollectLeft | 1.14 s | 0.84 s (-26%) | | (Int64, Utf8View) key + 1 NULL, CollectLeft | 1.31 s | 1.01 s (-23%) | | Utf8View key + 1 NULL, Partitioned | 729 ms | 715 ms | | Control: no NULL, CollectLeft | 802 ms | 809 ms | | Control: 2 rows per key, CollectLeft | 1.15 s | 1.12 s | With one NULL key, `join_time` now matches the no-NULL control. In `Partitioned` mode all NULL-key build rows hash to a single partition, so only that partition lost the fast path, and the end-to-end change there is within noise. Run-to-run spread on this shared machine is about ±3%, which covers every control. ## Are there any user-facing changes? No. `join_hash_map` is `#[doc(hidden)]` internal API, although the signatures of its free functions `update_from_iter` and `get_matched_indices_with_limit_offset` change.
1 parent a67d64a commit 1fa378d

3 files changed

Lines changed: 199 additions & 6 deletions

File tree

‎datafusion/physical-plan/src/joins/join_hash_map.rs‎

Lines changed: 132 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,18 +146,22 @@ pub struct JoinHashMapU32 {
146146
map: HashTable<(u64, u32)>,
147147
// Stores indices in chained list data structure
148148
next: Vec<u32>,
149+
// Whether any hash value maps to more than one row
150+
chained: bool,
149151
}
150152

151153
impl JoinHashMapU32 {
152154
#[cfg(test)]
153155
pub(crate) fn new(map: HashTable<(u64, u32)>, next: Vec<u32>) -> Self {
154-
Self { map, next }
156+
let chained = map.len() != next.len();
157+
Self { map, next, chained }
155158
}
156159

157160
pub fn with_capacity(cap: usize) -> Self {
158161
Self {
159162
map: HashTable::with_capacity(cap),
160163
next: vec![0; cap],
164+
chained: false,
161165
}
162166
}
163167
}
@@ -176,7 +180,8 @@ impl JoinHashMapType for JoinHashMapU32 {
176180
iter: Box<dyn Iterator<Item = (usize, &'a u64)> + Send + 'a>,
177181
deleted_offset: usize,
178182
) {
179-
update_from_iter::<u32>(&mut self.map, &mut self.next, iter, deleted_offset);
183+
self.chained |=
184+
update_from_iter::<u32>(&mut self.map, &mut self.next, iter, deleted_offset);
180185
}
181186

182187
fn get_matched_indices<'a>(
@@ -199,6 +204,7 @@ impl JoinHashMapType for JoinHashMapU32 {
199204
get_matched_indices_with_limit_offset::<u32>(
200205
&self.map,
201206
&self.next,
207+
!self.chained,
202208
hash_values,
203209
valid_keys,
204210
limit,
@@ -226,18 +232,22 @@ pub struct JoinHashMapU64 {
226232
map: HashTable<(u64, u64)>,
227233
// Stores indices in chained list data structure
228234
next: Vec<u64>,
235+
// Whether any hash value maps to more than one row
236+
chained: bool,
229237
}
230238

231239
impl JoinHashMapU64 {
232240
#[cfg(test)]
233241
pub(crate) fn new(map: HashTable<(u64, u64)>, next: Vec<u64>) -> Self {
234-
Self { map, next }
242+
let chained = map.len() != next.len();
243+
Self { map, next, chained }
235244
}
236245

237246
pub fn with_capacity(cap: usize) -> Self {
238247
Self {
239248
map: HashTable::with_capacity(cap),
240249
next: vec![0; cap],
250+
chained: false,
241251
}
242252
}
243253
}
@@ -256,7 +266,8 @@ impl JoinHashMapType for JoinHashMapU64 {
256266
iter: Box<dyn Iterator<Item = (usize, &'a u64)> + Send + 'a>,
257267
deleted_offset: usize,
258268
) {
259-
update_from_iter::<u64>(&mut self.map, &mut self.next, iter, deleted_offset);
269+
self.chained |=
270+
update_from_iter::<u64>(&mut self.map, &mut self.next, iter, deleted_offset);
260271
}
261272

262273
fn get_matched_indices<'a>(
@@ -279,6 +290,7 @@ impl JoinHashMapType for JoinHashMapU64 {
279290
get_matched_indices_with_limit_offset::<u64>(
280291
&self.map,
281292
&self.next,
293+
!self.chained,
282294
hash_values,
283295
valid_keys,
284296
limit,
@@ -304,15 +316,19 @@ impl JoinHashMapType for JoinHashMapU64 {
304316
use crate::joins::MapOffset;
305317
use crate::joins::chain::traverse_chain;
306318

319+
/// Inserts the rows of `iter` into `map` and `next`. Returns `true` if any row
320+
/// was chained onto an existing hash value.
307321
pub fn update_from_iter<'a, T>(
308322
map: &mut HashTable<(u64, T)>,
309323
next: &mut [T],
310324
iter: Box<dyn Iterator<Item = (usize, &'a u64)> + Send + 'a>,
311325
deleted_offset: usize,
312-
) where
326+
) -> bool
327+
where
313328
T: Copy + TryFrom<usize> + PartialOrd,
314329
<T as TryFrom<usize>>::Error: Debug,
315330
{
331+
let mut chained = false;
316332
for (row, &hash_value) in iter {
317333
let entry = map.entry(
318334
hash_value,
@@ -329,12 +345,14 @@ pub fn update_from_iter<'a, T>(
329345
*index = T::try_from(row + 1).unwrap();
330346
// Update chained Vec at `row` with previous value
331347
next[row - deleted_offset] = prev_index;
348+
chained = true;
332349
}
333350
Vacant(vacant_entry) => {
334351
vacant_entry.insert((hash_value, T::try_from(row + 1).unwrap()));
335352
}
336353
}
337354
}
355+
chained
338356
}
339357

340358
pub fn get_matched_indices<'a, T>(
@@ -385,10 +403,13 @@ where
385403
(input_indices, match_indices)
386404
}
387405

406+
/// `unique` must be `true` only if every hash value in `map` maps to a single
407+
/// row; the chain in `next_chain` is then not traversed.
388408
#[expect(clippy::too_many_arguments)]
389409
pub fn get_matched_indices_with_limit_offset<T>(
390410
map: &HashTable<(u64, T)>,
391411
next_chain: &[T],
412+
unique: bool,
392413
hash_values: &[u64],
393414
valid_keys: Option<&NullBuffer>,
394415
limit: usize,
@@ -408,7 +429,7 @@ where
408429

409430
// Check if hashmap consists of unique values
410431
// If so, we can skip the chain traversal
411-
if map.len() == next_chain.len() {
432+
if unique {
412433
let start = offset.0;
413434
let end = (start + limit).min(hash_values.len());
414435
for (i, &hash) in hash_values[start..end].iter().enumerate() {
@@ -569,4 +590,109 @@ mod tests {
569590
assert_eq!(input_indices, vec![1, 1]);
570591
assert_eq!(match_indices, vec![3, 1]);
571592
}
593+
594+
#[test]
595+
fn test_unique_fast_path_with_uninserted_row() {
596+
// Row 1 is left out, as `update_hash` does for a NULL key under
597+
// `NullEquality::NullEqualsNothing`. Every hash value still maps to a
598+
// single row, so the probe takes the unique-key fast path.
599+
let build_hashes = [10u64, 99, 20, 30];
600+
let mut hash_map = JoinHashMapU32::with_capacity(build_hashes.len());
601+
hash_map.update_from_iter(
602+
Box::new(build_hashes.iter().enumerate().filter(|(row, _)| *row != 1)),
603+
0,
604+
);
605+
606+
let probe_hashes = vec![10, 20, 30];
607+
let mut input_indices = vec![];
608+
let mut match_indices = vec![];
609+
let next_offset = hash_map.get_matched_indices_with_limit_offset(
610+
&probe_hashes,
611+
None,
612+
2,
613+
(0, None),
614+
&mut input_indices,
615+
&mut match_indices,
616+
);
617+
// The fast path limits by probe rows and resumes at probe row 2; the
618+
// chain walk would stop after two matches at `(1, Some(0))`.
619+
assert_eq!(next_offset, Some((2, None)));
620+
assert_eq!(input_indices, vec![0, 1]);
621+
assert_eq!(match_indices, vec![0, 2]);
622+
623+
let next_offset = hash_map.get_matched_indices_with_limit_offset(
624+
&probe_hashes,
625+
None,
626+
2,
627+
(2, None),
628+
&mut input_indices,
629+
&mut match_indices,
630+
);
631+
assert_eq!(next_offset, None);
632+
assert_eq!(input_indices, vec![2]);
633+
assert_eq!(match_indices, vec![3]);
634+
}
635+
636+
#[test]
637+
fn test_chain_from_earlier_update_disables_fast_path() {
638+
// Rows 0 and 1 share a hash value; a later update without a chain must
639+
// not re-enable the unique-key fast path.
640+
let mut hash_map = JoinHashMapU32::with_capacity(3);
641+
hash_map.update_from_iter(Box::new([10u64, 10u64].iter().enumerate()), 0);
642+
hash_map.update_from_iter(Box::new([(2, &20u64)].into_iter()), 0);
643+
644+
let mut input_indices = vec![];
645+
let mut match_indices = vec![];
646+
let next_offset = hash_map.get_matched_indices_with_limit_offset(
647+
&[10],
648+
None,
649+
8192,
650+
(0, None),
651+
&mut input_indices,
652+
&mut match_indices,
653+
);
654+
655+
assert_eq!(next_offset, None);
656+
assert_eq!(input_indices, vec![0, 0]);
657+
assert_eq!(match_indices, vec![1, 0]);
658+
}
659+
660+
#[test]
661+
fn test_chain_created_by_later_update_disables_fast_path() {
662+
assert_chain_created_by_later_update(JoinHashMapU32::with_capacity(3));
663+
assert_chain_created_by_later_update(JoinHashMapU64::with_capacity(3));
664+
}
665+
666+
/// Rows 0 and 2 share a hash value but arrive in separate updates, and row
667+
/// 1 is left out as a NULL key would be. The first update alone is unique;
668+
/// the second creates the chain. Probing with `limit = 1` must resume
669+
/// through the chain and return each build row exactly once.
670+
fn assert_chain_created_by_later_update(mut hash_map: impl JoinHashMapType) {
671+
hash_map.update_from_iter(Box::new([(0, &10u64)].into_iter()), 0);
672+
hash_map.update_from_iter(Box::new([(2, &10u64)].into_iter()), 0);
673+
674+
let mut input_indices = vec![];
675+
let mut match_indices = vec![];
676+
let mut matched = vec![];
677+
let mut offset = (0, None);
678+
for _ in 0..3 {
679+
let next_offset = hash_map.get_matched_indices_with_limit_offset(
680+
&[10],
681+
None,
682+
1,
683+
offset,
684+
&mut input_indices,
685+
&mut match_indices,
686+
);
687+
assert_eq!(input_indices, vec![0]);
688+
matched.extend_from_slice(&match_indices);
689+
match next_offset {
690+
Some(next) => offset = next,
691+
None => break,
692+
}
693+
}
694+
695+
matched.sort_unstable();
696+
assert_eq!(matched, vec![0, 2]);
697+
}
572698
}

‎datafusion/physical-plan/src/joins/stream_join_utils.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ impl JoinHashMapType for PruningJoinHashMap {
9292
get_matched_indices_with_limit_offset::<u64>(
9393
&self.map,
9494
&next,
95+
self.map.len() == next.len(),
9596
hash_values,
9697
valid_keys,
9798
limit,

‎datafusion/physical-plan/src/joins/utils.rs‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2864,6 +2864,72 @@ mod tests {
28642864
Ok(())
28652865
}
28662866

2867+
#[test]
2868+
fn update_hash_null_key_does_not_change_matches() -> Result<()> {
2869+
use crate::joins::join_hash_map::JoinHashMapU32;
2870+
2871+
let random_state = RandomState::with_seed(42);
2872+
let probe: ArrayRef = Arc::new(StringArray::from(vec!["c", "a", "x", "b"]));
2873+
let mut probe_hashes = vec![0; probe.len()];
2874+
create_hashes([&probe], &random_state, &mut probe_hashes)?;
2875+
2876+
// Probes a unique-key build side 2 rows at a time and returns the
2877+
// matched (probe row, build key) pairs.
2878+
let matches = |build: Vec<Option<&str>>| -> Result<Vec<(u32, String)>> {
2879+
let build = StringArray::from(build);
2880+
let schema =
2881+
Arc::new(Schema::new(vec![Field::new("k", DataType::Utf8, true)]));
2882+
let batch = RecordBatch::try_new(schema, vec![Arc::new(build.clone())])?;
2883+
let on: Vec<PhysicalExprRef> = vec![Arc::new(Column::new("k", 0))];
2884+
let mut map = JoinHashMapU32::with_capacity(batch.num_rows());
2885+
let mut hashes_buffer = vec![0; batch.num_rows()];
2886+
update_hash(
2887+
&on,
2888+
&batch,
2889+
&mut map,
2890+
0,
2891+
&random_state,
2892+
&mut hashes_buffer,
2893+
0,
2894+
true,
2895+
NullEquality::NullEqualsNothing,
2896+
)?;
2897+
let (mut input_indices, mut match_indices, mut pairs) =
2898+
(vec![], vec![], vec![]);
2899+
let mut offset = Some((0, None));
2900+
while let Some(current) = offset {
2901+
offset = map.get_matched_indices_with_limit_offset(
2902+
&probe_hashes,
2903+
None,
2904+
2,
2905+
current,
2906+
&mut input_indices,
2907+
&mut match_indices,
2908+
);
2909+
pairs.extend(
2910+
input_indices
2911+
.iter()
2912+
.zip(&match_indices)
2913+
.map(|(&p, &b)| (p, build.value(b as usize).to_string())),
2914+
);
2915+
}
2916+
pairs.sort();
2917+
Ok(pairs)
2918+
};
2919+
2920+
let without_null = matches(vec![Some("a"), Some("b"), Some("c")])?;
2921+
let with_null = matches(vec![Some("a"), None, Some("b"), Some("c")])?;
2922+
assert_eq!(with_null, without_null);
2923+
if cfg!(not(feature = "force_hash_collisions")) {
2924+
let expected = [(0, "c"), (1, "a"), (3, "b")];
2925+
let expected: Vec<_> =
2926+
expected.iter().map(|&(p, k)| (p, k.to_string())).collect();
2927+
assert_eq!(without_null, expected);
2928+
}
2929+
2930+
Ok(())
2931+
}
2932+
28672933
#[test]
28682934
fn get_anti_indices_handles_dense_matches() {
28692935
let input = UInt32Array::from(vec![2, 3, 4, 5]);

0 commit comments

Comments
 (0)