Skip to content

Commit e2e79ba

Browse files
committed
add cache to concat pointers to avoid un-needed allocations
1 parent 6ffcd94 commit e2e79ba

1 file changed

Lines changed: 35 additions & 6 deletions

File tree

  • datafusion/physical-plan/src/aggregates/group_values/multi_group_by

datafusion/physical-plan/src/aggregates/group_values/multi_group_by/dictionary.rs

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,15 @@ use datafusion_common::Result;
2525
use std::marker::PhantomData;
2626
use std::sync::Arc;
2727

28+
/// A [`GroupColumn`] implementation for dictionary arrays.
29+
/// wraps an inner [`GroupColumn`] that stores the dictionary values, and uses a null sentinel for null keys.
2830
pub struct DictionaryGroupValuesColumn<K: ArrowDictionaryKeyType + Send + Sync> {
2931
inner: Box<dyn GroupColumn>,
32+
// unary array used as a null sentinel for dictionary keys that are null
3033
null_array: ArrayRef,
3134
_phantom: PhantomData<K>,
35+
values_ptr: Option<ArrayRef>,
36+
combined_ptr: Option<ArrayRef>,
3237
}
3338

3439
impl<K: ArrowDictionaryKeyType + Send + Sync> DictionaryGroupValuesColumn<K> {
@@ -38,6 +43,8 @@ impl<K: ArrowDictionaryKeyType + Send + Sync> DictionaryGroupValuesColumn<K> {
3843
inner,
3944
null_array,
4045
_phantom: PhantomData,
46+
values_ptr: None,
47+
combined_ptr: None,
4148
}
4249
}
4350

@@ -115,9 +122,19 @@ impl<K: ArrowDictionaryKeyType + Send + Sync> GroupColumn
115122
equal_to_results,
116123
);
117124
} else {
118-
// Null keys must be routed to a null sentinel.
119-
let combined = concat(&[dict_values.as_ref(), self.null_array.as_ref()])
120-
.expect("concat of dict values and null sentinel should not fail");
125+
// Null keys must be routed to a null sentinel. A cheap ptr check
126+
// avoids an O(values) concat and allocation when the dictionary
127+
// array is the same across calls (populated by vectorized_append).
128+
let combined = {
129+
if let Some(values_ptr) = self.values_ptr.as_ref()
130+
&& Arc::ptr_eq(values_ptr, dict_values)
131+
{
132+
self.combined_ptr.clone().unwrap()
133+
} else {
134+
concat(&[dict_values.as_ref(), self.null_array.as_ref()])
135+
.expect("concat of dict values and null sentinel should not fail")
136+
}
137+
};
121138
let null_sentinel_index = combined.len() - 1;
122139
let value_indices: Vec<usize> = rhs_rows
123140
.iter()
@@ -143,9 +160,21 @@ impl<K: ArrowDictionaryKeyType + Send + Sync> GroupColumn
143160
.collect();
144161
self.inner.vectorized_append(dict.values(), &value_indices)
145162
} else {
146-
let combined_with_null_sentinel =
147-
concat(&[dict.values(), self.null_array.as_ref()])
148-
.expect("concat of dict values and null sentinel should not fail");
163+
let combined_with_null_sentinel = {
164+
if let Some(values_ptr) = self.values_ptr.as_ref()
165+
&& Arc::ptr_eq(values_ptr, dict.values())
166+
{
167+
self.combined_ptr.clone().unwrap()
168+
} else {
169+
let combined = concat(&[dict.values(), self.null_array.as_ref()])
170+
.expect(
171+
"concat of dict values and null sentinel should not fail",
172+
);
173+
self.values_ptr = Some(Arc::clone(dict.values()));
174+
self.combined_ptr = Some(Arc::clone(&combined));
175+
combined
176+
}
177+
};
149178
let null_sentinel_index = combined_with_null_sentinel.len() - 1;
150179
let value_indices: Vec<usize> = rows
151180
.iter()

0 commit comments

Comments
 (0)