dwsmith1983 commented on code in PR #26793:
URL: https://github.com/apache/datafusion/pull/26793#discussion_r4435047691


##########
datafusion/physical-plan/src/joins/hash_join/exec.rs:
##########
@@ -3217,28 +3216,38 @@ fn build_in_memory(
         // Arc is used instead of Box to allow sharing with 
SharedBuildAccumulator for hash map pushdown
         let mut hashmap = new_join_hashmap(num_rows, &mut reservation, 
metrics)?;
 
+        // Every slice shares the whole dictionary of a dictionary key, which
+        // `create_hashes` hashes on each call: a slice at least as long as the
+        // dictionary keeps that cost linear in the build rows.
+        let chunk_rows = left_values
+            .iter()
+            .filter_map(|values| values.as_any_dictionary_opt())
+            .map(|dict| dict.values().len())
+            .fold(max_batch_rows, usize::max);

Review Comment:
   Thanks @jayzhan211, applied in 8a4222e9a with `unsliced_hash_len` walking 
the key's `ArrayData` as you suggested, and your test is in. Two small 
additions:
   
   - Below a dictionary or list view, the whole subtree is hashed on each call, 
not only its direct values: for `Dictionary<Int32, List<Int32>>`, 
`hash_dictionary` hashes the whole list values array, so `hash_list_array` 
covers every element. A `subtree_max_len` helper counts the longest array under 
those nodes. The test gained rows for that case (it failed at 2 instead of 7 
before), a sliced plain list, a sliced `FixedSizeList` and a sliced 
`List<Dictionary>`.
   - `chunk_rows` is capped at the build row count, so a dictionary much longer 
than the build side no longer inflates the prepared scratch reservation.
   
   The `HashJoinExec` doc and the loop comment are updated to match. I also 
merged main.
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to