Skip to content

Commit cf8d870

Browse files
committed
suggestion by review
1 parent 8f229a6 commit cf8d870

2 files changed

Lines changed: 25 additions & 10 deletions

File tree

datafusion/functions-nested/src/resize.rs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,25 +209,27 @@ fn general_list_resize<O: OffsetSizeTrait + TryInto<i64>>(
209209
// Track the largest per-row growth so the uniform-fill fast path can
210210
// materialize one reusable fill buffer of the required size.
211211
let mut max_extra: usize = 0;
212+
let mut output_values_len: usize = 0;
212213
for (row_index, offset_window) in array.offsets().windows(2).enumerate() {
213214
if array.is_null(row_index) {
214215
continue;
215216
}
216217
let target_count = count_array.value(row_index).to_usize().ok_or_else(|| {
217218
internal_datafusion_err!("array_resize: failed to convert size to usize")
218219
})?;
220+
output_values_len =
221+
output_values_len.checked_add(target_count).ok_or_else(|| {
222+
internal_datafusion_err!("array_resize: output size overflow")
223+
})?;
219224
let current_len = (offset_window[1] - offset_window[0]).to_usize().unwrap();
220225
if target_count > current_len {
221-
let extra = target_count - current_len;
222-
if extra > max_extra {
223-
max_extra = extra;
224-
}
226+
max_extra = max_extra.max(target_count - current_len);
225227
}
226228
}
227229

228230
// The fast path is valid when at least one row grows and every row would
229231
// use the same fill value.
230-
let is_uniform_fill = max_extra > 0
232+
let use_bulk_fill = max_extra > 0
231233
&& match &default_element {
232234
None => true,
233235
Some(fill_array) => {
@@ -246,7 +248,7 @@ fn general_list_resize<O: OffsetSizeTrait + TryInto<i64>>(
246248

247249
// Fast path: at least one row needs to grow and all rows share
248250
// the same fill value.
249-
if is_uniform_fill {
251+
if use_bulk_fill {
250252
let fill_scalar = match &default_element {
251253
None => ScalarValue::try_from(&data_type)?,
252254
Some(fill_array) if fill_array.logical_null_count() == fill_array.len() => {
@@ -257,7 +259,7 @@ fn general_list_resize<O: OffsetSizeTrait + TryInto<i64>>(
257259
let default_element = fill_scalar.to_array_of_size(max_extra)?;
258260
let default_value_data = default_element.to_data();
259261

260-
let capacity = Capacities::Array(original_data.len() + default_value_data.len());
262+
let capacity = Capacities::Array(output_values_len);
261263
let mut offsets = vec![O::usize_as(0)];
262264
let mut mutable = MutableArrayData::with_capacities(
263265
vec![&original_data, &default_value_data],
@@ -283,11 +285,11 @@ fn general_list_resize<O: OffsetSizeTrait + TryInto<i64>>(
283285
if start + count > offset_window[1] {
284286
let extra_count = (start + count - offset_window[1]).to_usize().unwrap();
285287
let end = offset_window[1];
286-
mutable.extend(0, (start).to_usize().unwrap(), (end).to_usize().unwrap());
288+
mutable.extend(0, start.to_usize().unwrap(), end.to_usize().unwrap());
287289
mutable.extend(1, 0, extra_count);
288290
} else {
289291
let end = start + count;
290-
mutable.extend(0, (start).to_usize().unwrap(), (end).to_usize().unwrap());
292+
mutable.extend(0, start.to_usize().unwrap(), end.to_usize().unwrap());
291293
};
292294
offsets.push(offsets[row_index] + count);
293295
}
@@ -312,7 +314,7 @@ fn general_list_resize<O: OffsetSizeTrait + TryInto<i64>>(
312314
let default_value_data = default_element.to_data();
313315

314316
// create a mutable array to store the original data
315-
let capacity = Capacities::Array(original_data.len() + default_value_data.len());
317+
let capacity = Capacities::Array(output_values_len);
316318
let mut offsets = vec![O::usize_as(0)];
317319
let mut mutable = MutableArrayData::with_capacities(
318320
vec![&original_data, &default_value_data],

datafusion/sqllogictest/test_files/array.slt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8860,6 +8860,19 @@ NULL
88608860
[51, 52, 53, 54, 55, NULL, 57, 58, 59, 60, NULL, NULL, NULL]
88618861
[61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 7, 7, 7, 7, 7]
88628862

8863+
# array_resize columnar test #3
8864+
query ?
8865+
select array_resize(column1, column2, 9) from array_resize_values;
8866+
----
8867+
[1, NULL]
8868+
[11, 12, NULL, 14, 15]
8869+
[21, 22, 23, 24, NULL, 26, 27, 28]
8870+
[31, 32, 33, 34, 35, 36, NULL, 38, 39, 40, 9, 9]
8871+
NULL
8872+
[]
8873+
[51, 52, 53, 54, 55, NULL, 57, 58, 59, 60, 9, 9, 9]
8874+
[61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 9, 9, 9, 9, 9]
8875+
88638876
## array_reverse
88648877
query ??
88658878
select array_reverse(make_array(1, 2, 3)), array_reverse(make_array(1));

0 commit comments

Comments
 (0)