jayzhan211 commented on code in PR #25602:
URL: https://github.com/apache/datafusion/pull/25602#discussion_r4083303908


##########
datafusion/physical-plan/src/joins/hash_join/exec.rs:
##########
@@ -3094,19 +3097,41 @@ async fn collect_left_input(
             .iter()
             .map(|arr| arr.get_array_memory_size())
             .sum::<usize>();
-        if left_values.is_empty()
-            || left_values[0].is_empty()
-            || estimated_size > 
config.optimizer.hash_join_inlist_pushdown_max_size
-            || map.num_of_distinct_key()
-                > config
+
+        let pushdown_inlist = !left_values.is_empty()
+            && !left_values[0].is_empty()
+            && estimated_size <= 
config.optimizer.hash_join_inlist_pushdown_max_size
+            && map.num_of_distinct_key()
+                <= config
                     .optimizer
-                    .hash_join_inlist_pushdown_max_distinct_values
+                    .hash_join_inlist_pushdown_max_distinct_values;
+
+        if pushdown_inlist
+            && let Some(in_list_values) = 
build_struct_inlist_values(&left_values)?
         {
-            PushdownStrategy::Map(Arc::clone(&map))
-        } else if let Some(in_list_values) = 
build_struct_inlist_values(&left_values)? {
             PushdownStrategy::InList(in_list_values)
         } else {
-            PushdownStrategy::Map(Arc::clone(&map))
+            // Past the InList threshold use a bucket bitmap for container 
pruning.
+            let pruning_bitmap = match (left_values.as_slice(), 
bounds.as_ref()) {
+                ([keys], Some(bounds)) if !keys.is_empty() => bounds
+                    .get_column_bounds(0)
+                    .and_then(|b| {
+                        KeyRangeBitmap::try_new(
+                            keys,
+                            &b.min,
+                            &b.max,
+                            map.num_of_distinct_key(),
+                        )
+                    })
+                    .map(Arc::new),
+                _ => None,
+            };
+            if let Some(bitmap) = pruning_bitmap.as_ref() {
+                // Held for the join's lifetime, so charge it like the maps.
+                reservation.try_grow(bitmap.size())?;
+                metrics.build_mem_used.add(bitmap.size());
+            }

Review Comment:
   The bitmap is only a pruning aid, but `try_grow(...)?` turns a failed 
reservation into a query error. A join with less than 128 KiB of headroom per 
build partition used to succeed and now fails with ResourcesExhausted. Drop the 
bitmap instead:
   
   ```suggestion
               // Held for the join's lifetime, so charge it like the maps; it 
is
               // optional, so skip it rather than fail when the pool is full.
               let pruning_bitmap = pruning_bitmap.filter(|bitmap| {
                   let ok = reservation.try_grow(bitmap.size()).is_ok();
                   if ok {
                       metrics.build_mem_used.add(bitmap.size());
                   }
                   ok
               });
   ```



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