Skip to content

Commit 32bde44

Browse files
committed
forbid reserve() to prevent panic in clippy
1 parent acb80ce commit 32bde44

8 files changed

Lines changed: 58 additions & 10 deletions

File tree

clippy.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
disallowed-methods = [
22
{ path = "tokio::task::spawn", reason = "To provide cancel-safety, use `SpawnedTask::spawn` instead (https://github.com/apache/datafusion/issues/6513)" },
33
{ path = "tokio::task::spawn_blocking", reason = "To provide cancel-safety, use `SpawnedTask::spawn_blocking` instead (https://github.com/apache/datafusion/issues/6513)" },
4+
{ path = "std::vec::Vec::reserve", reason = "Use `Vec::try_reserve` so allocation failures can be reported instead of panicking", replacement = "try_reserve" },
45
]
56

67
disallowed-types = [

datafusion/functions-aggregate/src/median.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ use arrow::datatypes::{
4141

4242
use datafusion_common::types::{NativeType, logical_float64};
4343
use datafusion_common::{
44-
DataFusionError, Result, ScalarValue, assert_eq_or_internal_err,
44+
DataFusionError, Result, ScalarValue, assert_eq_or_internal_err, exec_datafusion_err,
4545
internal_datafusion_err,
4646
};
4747
use datafusion_expr::function::StateFieldsArgs;
@@ -282,7 +282,12 @@ impl<T: ArrowNumericType> Accumulator for MedianAccumulator<T> {
282282

283283
fn update_batch(&mut self, values: &[ArrayRef]) -> Result<()> {
284284
let values = values[0].as_primitive::<T>();
285-
self.all_values.reserve(values.len() - values.null_count());
285+
let additional = values.len() - values.null_count();
286+
self.all_values.try_reserve(additional).map_err(|e| {
287+
exec_datafusion_err!(
288+
"failed to reserve {additional} values for median accumulator: {e}"
289+
)
290+
})?;
286291
self.all_values.extend(values.iter().flatten());
287292
Ok(())
288293
}

datafusion/functions-aggregate/src/percentile_cont.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ use datafusion_functions_aggregate_common::noop_accumulator::NoopAccumulator;
3838

3939
use crate::min_max::{max_udaf, min_udaf};
4040
use datafusion_common::{
41-
Result, ScalarValue, internal_datafusion_err, utils::take_function_args,
41+
Result, ScalarValue, exec_datafusion_err, internal_datafusion_err,
42+
utils::take_function_args,
4243
};
4344
use datafusion_expr::utils::format_state_name;
4445
use datafusion_expr::{
@@ -420,7 +421,12 @@ where
420421

421422
fn update_batch(&mut self, values: &[ArrayRef]) -> Result<()> {
422423
let values = values[0].as_primitive::<T>();
423-
self.all_values.reserve(values.len() - values.null_count());
424+
let additional = values.len() - values.null_count();
425+
self.all_values.try_reserve(additional).map_err(|e| {
426+
exec_datafusion_err!(
427+
"failed to reserve {additional} values for percentile_cont accumulator: {e}"
428+
)
429+
})?;
424430
self.all_values.extend(values.iter().flatten());
425431
Ok(())
426432
}

datafusion/functions/src/string/common.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,10 @@ fn case_conversion_utf8view_ascii_inner<F: Fn(&u8) -> u8>(
520520
block_size = block_size.saturating_mul(2);
521521
}
522522
let to_reserve = len.max(block_size as usize);
523+
#[expect(
524+
clippy::disallowed_methods,
525+
reason = "StringView block_size bounds growth, so reserve cannot overflow capacity"
526+
)]
523527
in_progress.reserve(to_reserve);
524528
}
525529

datafusion/functions/src/strings.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -703,6 +703,10 @@ impl StringViewArrayBuilder {
703703
if self.in_progress.capacity() < required_cap {
704704
self.flush_in_progress();
705705
let to_reserve = (length as usize).max(self.next_block_size() as usize);
706+
#[expect(
707+
clippy::disallowed_methods,
708+
reason = "StringView block_size bounds growth, so reserve cannot overflow capacity"
709+
)]
706710
self.in_progress.reserve(to_reserve);
707711
}
708712

@@ -730,6 +734,10 @@ impl StringViewArrayBuilder {
730734
if self.in_progress.capacity() < required_cap {
731735
self.flush_in_progress();
732736
let to_reserve = (length as usize).max(self.next_block_size() as usize);
737+
#[expect(
738+
clippy::disallowed_methods,
739+
reason = "StringView block_size bounds growth, so reserve cannot overflow capacity"
740+
)]
733741
self.in_progress.reserve(to_reserve);
734742
}
735743
}

datafusion/physical-plan/src/recursive_query.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ use arrow::datatypes::{Field, Schema, SchemaRef};
3939
use arrow::record_batch::RecordBatch;
4040
use datafusion_common::tree_node::TreeNodeRecursion;
4141
use datafusion_common::tree_node::{Transformed, TransformedResult, TreeNode};
42-
use datafusion_common::{Result, internal_datafusion_err, not_impl_err};
42+
use datafusion_common::{
43+
Result, exec_datafusion_err, internal_datafusion_err, not_impl_err,
44+
};
4345
use datafusion_execution::TaskContext;
4446
use datafusion_execution::memory_pool::{MemoryConsumer, MemoryReservation};
4547
use datafusion_physical_expr::PhysicalExpr;
@@ -489,7 +491,14 @@ impl DistinctDeduplicator {
489491
/// We also detect duplicates by enforcing that group ids are increasing.
490492
fn deduplicate(&mut self, batch: &RecordBatch) -> Result<RecordBatch> {
491493
let size_before = self.group_values.len();
492-
self.intern_output_buffer.reserve(batch.num_rows());
494+
let additional = batch.num_rows();
495+
self.intern_output_buffer
496+
.try_reserve(additional)
497+
.map_err(|e| {
498+
exec_datafusion_err!(
499+
"failed to reserve {additional} recursive query group ids: {e}"
500+
)
501+
})?;
493502
self.group_values
494503
.intern(batch.columns(), &mut self.intern_output_buffer)?;
495504
let mask = new_groups_mask(&self.intern_output_buffer, size_before);

datafusion/spark/src/function/math/hex.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use datafusion_common::utils::take_function_args;
3131
use datafusion_common::{
3232
DataFusionError,
3333
cast::{as_binary_array, as_fixed_size_binary_array, as_int64_array},
34-
exec_err,
34+
exec_datafusion_err, exec_err,
3535
};
3636
use datafusion_expr::{
3737
Coercion, ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignature,
@@ -178,7 +178,15 @@ where
178178
if let Some(b) = v {
179179
let bytes = b.as_ref();
180180
buffer.clear();
181-
buffer.reserve(bytes.len() * 2);
181+
let additional = bytes
182+
.len()
183+
.checked_mul(2)
184+
.ok_or_else(|| exec_datafusion_err!("hex output size overflow"))?;
185+
buffer.try_reserve(additional).map_err(|e| {
186+
exec_datafusion_err!(
187+
"failed to reserve {additional} bytes for hex output: {e}"
188+
)
189+
})?;
182190
for &byte in bytes {
183191
buffer.extend_from_slice(&lookup[byte as usize]);
184192
}

datafusion/spark/src/function/math/unhex.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ use datafusion_common::cast::{
2222
};
2323
use datafusion_common::types::logical_string;
2424
use datafusion_common::utils::take_function_args;
25-
use datafusion_common::{DataFusionError, Result, ScalarValue, exec_err};
25+
use datafusion_common::{
26+
DataFusionError, Result, ScalarValue, exec_datafusion_err, exec_err,
27+
};
2628
use datafusion_expr::{
2729
Coercion, ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature,
2830
TypeSignatureClass, Volatility,
@@ -125,7 +127,12 @@ where
125127
for v in iter {
126128
if let Some(s) = v {
127129
buffer.clear();
128-
buffer.reserve(s.as_ref().len().div_ceil(2));
130+
let additional = s.as_ref().len().div_ceil(2);
131+
buffer.try_reserve(additional).map_err(|e| {
132+
exec_datafusion_err!(
133+
"failed to reserve {additional} bytes for unhex output: {e}"
134+
)
135+
})?;
129136
if unhex_common(s.as_ref().as_bytes(), &mut buffer) {
130137
builder.append_value(&buffer);
131138
} else {

0 commit comments

Comments
 (0)