This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 000a7fd32e Enable five more clippy lints (#10935)
000a7fd32e is described below
commit 000a7fd32ecf928f9505afa3d77786179f8fd8dd
Author: Emil Ernerfeldt <[email protected]>
AuthorDate: Tue Sep 1 17:09:02 2026 +0200
Enable five more clippy lints (#10935)
# Which issue does this PR close?
No issue in particular
- Follow-up to #10673
- Follow-up to #10721
- Follow-up to #10742
# Rationale for this change
#10742 left the `pedantic` lints we violate as `allow`, with a violation
count each. These five are the ones worth paying for: two guard raw
pointers, one guards bindings that claim to be unused, and two improve
readability and test output.
## What changes are included in this PR?
One commit per new lint (easiest to review commit by commit!):
*
[`redundant_else`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_else)
*
[`manual_assert_eq`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert_eq)
*
[`borrow_as_ptr`](https://rust-lang.github.io/rust-clippy/master/index.html#borrow_as_ptr)
- the FFI stream code now builds its pointers with `&raw mut` instead of
going through a `&mut`
*
[`used_underscore_binding`](https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding)
- two conditionally-used parameters keep their names and get a
feature-gated `expect`
*
[`cast_ptr_alignment`](https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment)
- the five byte-pointer casts now say why they are sound
Let me know if you disagree with any of them.
## Are these changes tested?
Covered by existing tests plus the clippy CI job.
## Are there any user-facing changes?
No.
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
Cargo.toml | 5 --
arrow-array/src/array/null_array.rs | 2 +-
arrow-array/src/ffi.rs | 13 ++++--
arrow-array/src/ffi_stream.rs | 13 +++---
arrow-avro/src/reader/mod.rs | 8 ++--
arrow-buffer/src/bigint/mod.rs | 4 +-
arrow-buffer/src/buffer/mutable.rs | 4 +-
arrow-buffer/src/util/bit_mask.rs | 4 ++
arrow-buffer/src/util/bit_util.rs | 4 ++
arrow-data/src/equal/boolean.rs | 10 ++--
.../src/bin/arrow-json-integration-test.rs | 4 +-
arrow-ipc/src/writer.rs | 8 ++--
arrow-select/src/interleave.rs | 2 +-
arrow-select/src/merge.rs | 5 +-
arrow-select/src/take.rs | 8 ++--
arrow-select/src/union_extract.rs | 3 +-
arrow/src/util/test_util.rs | 10 ++--
parquet-geospatial/src/interval.rs | 8 ++--
parquet-variant-compute/src/arrow_to_variant.rs | 14 +++---
parquet-variant/tests/variant_interop.rs | 5 +-
.../src/arrow/arrow_reader/selection/selector.rs | 3 +-
parquet/src/arrow/arrow_writer/levels.rs | 2 +-
parquet/src/arrow/buffer/dictionary_buffer.rs | 14 +++++-
parquet/src/arrow/in_memory_row_group.rs | 4 +-
parquet/src/column/writer/mod.rs | 30 ++++++------
parquet/src/compression.rs | 7 +++
parquet/src/data_type.rs | 8 ++--
parquet/src/file/metadata/reader.rs | 22 ++++-----
parquet/src/file/statistics.rs | 54 ++++++++++++++--------
parquet/src/geospatial/accumulator.rs | 7 +++
parquet/src/schema/parser.rs | 5 +-
parquet/src/util/bit_util.rs | 2 +-
parquet/tests/encryption/encryption_async.rs | 54 +++++++++++-----------
33 files changed, 195 insertions(+), 151 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index df851b3ccb..db88598802 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -220,12 +220,10 @@ wildcard_dependencies = "warn"
# Lints in `pedantic` that we currently allow.
# Some of these we should consider enabling in the future.
# Counts come from `cargo clippy --all-features --all-targets`.
-borrow_as_ptr = "allow" # ~9 violations in ~1 file
cast_lossless = "allow" # ~640 violations in ~100 files
cast_possible_truncation = "allow" # ~890 violations in ~190 files
cast_possible_wrap = "allow" # ~570 violations in ~120 files
cast_precision_loss = "allow" # ~130 violations in ~33 files
-cast_ptr_alignment = "allow" # ~3 violations in ~3 files
cast_sign_loss = "allow" # ~590 violations in ~150 files
default_trait_access = "allow" # ~210 violations in ~68 files
doc_markdown = "allow" # ~2300 violations in ~290 files
@@ -238,7 +236,6 @@ implicit_hasher = "allow" # ~9
violations in ~4 files
inline_always = "allow" # ~30 violations in ~17 files
items_after_statements = "allow" # ~130 violations in ~41 files
manual_assert = "allow" # ~20 violations in ~12 files
-manual_assert_eq = "allow" # ~29 violations in ~10 files
many_single_char_names = "allow" # ~8 violations in ~5 files
map_unwrap_or = "allow" # ~76 violations in ~40 files
match_bool = "allow" # ~100 violations in ~49 files
@@ -253,7 +250,6 @@ needless_pass_by_value = "allow" # ~260
violations in ~110 files
non_std_lazy_statics = "allow" # ~56 violations in ~10 files
range_plus_one = "allow" # ~17 violations in ~13 files
redundant_closure_for_method_calls = "allow" # ~310 violations in ~130 files
-redundant_else = "allow" # ~23 violations in ~16 files
return_self_not_must_use = "allow" # ~470 violations in ~91 files
semicolon_if_nothing_returned = "allow" # ~1300 violations in ~250 files
similar_names = "allow" # ~150 violations in ~43 files
@@ -267,7 +263,6 @@ unnecessary_debug_formatting = "allow" # ~23
violations in ~7 files
unnecessary_wraps = "allow" # ~64 violations in ~39 files
unreadable_literal = "allow" # ~4100 violations in ~120 files
unused_self = "allow" # ~31 violations in ~20 files
-used_underscore_binding = "allow" # ~15 violations in ~3 files
used_underscore_items = "allow" # ~120 violations in ~17 files
verbose_bit_mask = "allow" # 1 violation in 1 file
wildcard_imports = "allow" # ~230 violations in ~130 files
diff --git a/arrow-array/src/array/null_array.rs
b/arrow-array/src/array/null_array.rs
index feced56a14..00aa20a1ba 100644
--- a/arrow-array/src/array/null_array.rs
+++ b/arrow-array/src/array/null_array.rs
@@ -237,6 +237,6 @@ mod tests {
let data = mutable.freeze();
let struct_array = Arc::new(StructArray::from(data.clone()));
- assert!(make_array(data) == struct_array);
+ assert_eq!(&make_array(data), &(struct_array as ArrayRef));
}
}
diff --git a/arrow-array/src/ffi.rs b/arrow-array/src/ffi.rs
index 14235631a8..07707a641a 100644
--- a/arrow-array/src/ffi.rs
+++ b/arrow-array/src/ffi.rs
@@ -347,7 +347,7 @@ impl ImportedArrowArray<'_> {
| DataType::LargeListView(field)
| DataType::Map(field, _) => Ok([self.consume_child(0,
field.data_type())?].to_vec()),
DataType::Struct(fields) => {
- assert!(fields.len() == self.array.num_children());
+ assert_eq!(fields.len(), self.array.num_children());
fields
.iter()
.enumerate()
@@ -355,7 +355,7 @@ impl ImportedArrowArray<'_> {
.collect::<Result<Vec<_>>>()
}
DataType::Union(union_fields, _) => {
- assert!(union_fields.len() == self.array.num_children());
+ assert_eq!(union_fields.len(), self.array.num_children());
union_fields
.iter()
.enumerate()
@@ -397,7 +397,14 @@ impl ImportedArrowArray<'_> {
} else {
let lengths = self.array.buffer(self.array.num_buffers() - 1);
// SAFETY: is lengths is non-null, then it must be valid for
up to num_variadic_buffers.
- unsafe { std::slice::from_raw_parts(lengths.cast::<i64>(),
num_variadic_buffers) }
+ // The C data interface requires buffers to be aligned for
their type.
+ #[expect(
+ clippy::cast_ptr_alignment,
+ reason = "the C data interface requires aligned buffers"
+ )]
+ unsafe {
+ std::slice::from_raw_parts(lengths.cast::<i64>(),
num_variadic_buffers)
+ }
}
} else {
&[]
diff --git a/arrow-array/src/ffi_stream.rs b/arrow-array/src/ffi_stream.rs
index 58e23888f1..4bb570d56a 100644
--- a/arrow-array/src/ffi_stream.rs
+++ b/arrow-array/src/ffi_stream.rs
@@ -368,7 +368,7 @@ unsafe fn producer_error(stream_ptr: *mut
FFI_ArrowArrayStream) -> Option<String
fn get_stream_schema(stream_ptr: *mut FFI_ArrowArrayStream) ->
Result<SchemaRef> {
let mut schema = FFI_ArrowSchema::empty();
- let ret_code = unsafe { (*stream_ptr).get_schema.unwrap()(stream_ptr, &mut
schema) };
+ let ret_code = unsafe { (*stream_ptr).get_schema.unwrap()(stream_ptr, &raw
mut schema) };
if ret_code == 0 {
let schema = Schema::try_from(&schema)?;
@@ -395,7 +395,7 @@ impl ArrowArrayStreamReader {
));
}
- let schema = get_stream_schema(&mut stream)?;
+ let schema = get_stream_schema(&raw mut stream)?;
Ok(Self { stream, schema })
}
@@ -421,7 +421,8 @@ impl Iterator for ArrowArrayStreamReader {
fn next(&mut self) -> Option<Self::Item> {
let mut array = FFI_ArrowArray::empty();
- let ret_code = unsafe { self.stream.get_next.unwrap()(&mut
self.stream, &mut array) };
+ let ret_code =
+ unsafe { self.stream.get_next.unwrap()(&raw mut self.stream, &raw
mut array) };
if ret_code == 0 {
// The end of stream has been reached
@@ -445,7 +446,7 @@ impl Iterator for ArrowArrayStreamReader {
format!("Cannot get next batch from input stream. Error code:
{ret_code}");
// SAFETY: `self.stream` is valid and unreleased by construction,
and the
// `get_next` call above returned a non-zero code.
- let message = match unsafe { producer_error(&mut self.stream) } {
+ let message = match unsafe { producer_error(&raw mut self.stream)
} {
Some(producer_message) => format!("{message}. Producer error:
{producer_message}"),
None => message,
};
@@ -508,7 +509,7 @@ mod tests {
// Get schema from `FFI_ArrowArrayStream`
let mut ffi_schema = FFI_ArrowSchema::empty();
- let ret_code = unsafe { get_schema(&mut ffi_stream, &mut ffi_schema) };
+ let ret_code = unsafe { get_schema(&raw mut ffi_stream, &raw mut
ffi_schema) };
assert_eq!(ret_code, 0);
let exported_schema = Schema::try_from(&ffi_schema).unwrap();
@@ -518,7 +519,7 @@ mod tests {
let mut produced_batches = vec![];
loop {
let mut ffi_array = FFI_ArrowArray::empty();
- let ret_code = unsafe { get_next(&mut ffi_stream, &mut ffi_array)
};
+ let ret_code = unsafe { get_next(&raw mut ffi_stream, &raw mut
ffi_array) };
assert_eq!(ret_code, 0);
// The end of stream has been reached
diff --git a/arrow-avro/src/reader/mod.rs b/arrow-avro/src/reader/mod.rs
index 3b97b9bd48..f05df596a8 100644
--- a/arrow-avro/src/reader/mod.rs
+++ b/arrow-avro/src/reader/mod.rs
@@ -1706,10 +1706,9 @@ mod test {
if (n & !0x7F) == 0 {
out.push(n as u8);
break;
- } else {
- out.push(((n & 0x7F) | 0x80) as u8);
- n >>= 7;
}
+ out.push(((n & 0x7F) | 0x80) as u8);
+ n >>= 7;
}
out
}
@@ -3117,9 +3116,8 @@ mod test {
test.name
);
continue;
- } else {
- panic!("Test '{}' failed during build: {e}",
test.name);
}
+ panic!("Test '{}' failed during build: {e}", test.name);
}
};
let stream = Box::pin(stream::once(async { Bytes::from(body) }));
diff --git a/arrow-buffer/src/bigint/mod.rs b/arrow-buffer/src/bigint/mod.rs
index 7597e19896..2a359df2c1 100644
--- a/arrow-buffer/src/bigint/mod.rs
+++ b/arrow-buffer/src/bigint/mod.rs
@@ -732,14 +732,14 @@ impl i256 {
// self is between 10^64 and 10^77 (~i256::MAX).
// `value` is 14 digits max (10^77 / 10^64 = 10^13),
// so it fits to `low` u128
- debug_assert!(value.high == 0);
+ debug_assert_eq!(value.high, 0);
Some(64 + value.low.checked_ilog10()?)
} else if self >= POW10_32 {
let value = self.checked_div(POW10_32)?;
// self is between 10^32 and 10^64.
// `value` is 33 digits max (10^64/10^32=10^32)
// so it fits to `low` 128-bit value
- debug_assert!(value.high == 0);
+ debug_assert_eq!(value.high, 0);
Some(32 + value.low.checked_ilog10()?)
} else {
// self fits within u128 (high == 0 and self > 0).
diff --git a/arrow-buffer/src/buffer/mutable.rs
b/arrow-buffer/src/buffer/mutable.rs
index 150fe44043..c53cf3fcab 100644
--- a/arrow-buffer/src/buffer/mutable.rs
+++ b/arrow-buffer/src/buffer/mutable.rs
@@ -1465,13 +1465,13 @@ mod tests {
buf.extend_from_slice(&[0xaa]);
buf2.extend_from_slice(&[0xaa, 0xbb]);
- assert!(buf != buf2);
+ assert_ne!(buf, buf2);
buf.extend_from_slice(&[0xbb]);
assert_eq!(buf, buf2);
buf2.reserve(65);
- assert!(buf != buf2);
+ assert_ne!(buf, buf2);
}
#[test]
diff --git a/arrow-buffer/src/util/bit_mask.rs
b/arrow-buffer/src/util/bit_mask.rs
index b1f25eb014..d65988dca1 100644
--- a/arrow-buffer/src/util/bit_mask.rs
+++ b/arrow-buffer/src/util/bit_mask.rs
@@ -156,6 +156,10 @@ unsafe fn read_bytes_to_u64(data: &[u8], offset: usize,
count: usize) -> u64 {
/// The caller must ensure `data` has `offset..(offset + 8)` range
#[inline]
unsafe fn write_u64_bytes(data: &mut [u8], offset: usize, chunk: u64) {
+ #[expect(
+ clippy::cast_ptr_alignment,
+ reason = "the pointer is only written through `write_unaligned`"
+ )]
let ptr = unsafe { data.as_mut_ptr().add(offset) }.cast::<u64>();
unsafe { ptr.write_unaligned(chunk) };
}
diff --git a/arrow-buffer/src/util/bit_util.rs
b/arrow-buffer/src/util/bit_util.rs
index f06e03ef13..6faec24111 100644
--- a/arrow-buffer/src/util/bit_util.rs
+++ b/arrow-buffer/src/util/bit_util.rs
@@ -574,6 +574,10 @@ impl<'a> U64UnalignedSlice<'a> {
assert!(u64_len_in_bytes <= left_buffer_mut.len());
let (bytes_for_u64, remainder) =
left_buffer_mut.split_at_mut(u64_len_in_bytes);
+ #[expect(
+ clippy::cast_ptr_alignment,
+ reason = "`U64UnalignedSlice` only reads and writes through the
unaligned methods"
+ )]
let ptr = bytes_for_u64.as_mut_ptr().cast::<u64>();
let this = Self {
diff --git a/arrow-data/src/equal/boolean.rs b/arrow-data/src/equal/boolean.rs
index cbd18ad789..b413ddc3cb 100644
--- a/arrow-data/src/equal/boolean.rs
+++ b/arrow-data/src/equal/boolean.rs
@@ -59,12 +59,12 @@ pub(super) fn boolean_equal(
let rem = len % 8;
if rem == 0 {
return true;
- } else {
- let aligned_bits = len - rem;
- lhs_start += aligned_bits;
- rhs_start += aligned_bits;
- len = rem
}
+
+ let aligned_bits = len - rem;
+ lhs_start += aligned_bits;
+ rhs_start += aligned_bits;
+ len = rem;
}
equal_bits(
diff --git a/arrow-integration-testing/src/bin/arrow-json-integration-test.rs
b/arrow-integration-testing/src/bin/arrow-json-integration-test.rs
index a71ba166ca..3ee551fe4c 100644
--- a/arrow-integration-testing/src/bin/arrow-json-integration-test.rs
+++ b/arrow-integration-testing/src/bin/arrow-json-integration-test.rs
@@ -161,8 +161,8 @@ fn validate(arrow_name: &str, json_name: &str, verbose:
bool) -> Result<()> {
if let Some(Ok(arrow_batch)) = arrow_reader.next() {
// compare batches
let num_columns = arrow_batch.num_columns();
- assert!(num_columns == json_batch.num_columns());
- assert!(arrow_batch.num_rows() == json_batch.num_rows());
+ assert_eq!(num_columns, json_batch.num_columns());
+ assert_eq!(arrow_batch.num_rows(), json_batch.num_rows());
for i in 0..num_columns {
assert_eq!(
diff --git a/arrow-ipc/src/writer.rs b/arrow-ipc/src/writer.rs
index 11a3030e03..99d40632f2 100644
--- a/arrow-ipc/src/writer.rs
+++ b/arrow-ipc/src/writer.rs
@@ -1549,11 +1549,11 @@ fn compare_dictionaries(old: &ArrayData, new:
&ArrayData) -> DictionaryCompariso
let existing_len = old.len();
let new_len = new.len();
if existing_len == new_len {
- if *old == *new {
- return DictionaryComparison::Equal;
+ return if *old == *new {
+ DictionaryComparison::Equal
} else {
- return DictionaryComparison::NotEqual;
- }
+ DictionaryComparison::NotEqual
+ };
}
// Can't be a delta if the new is shorter than the existing
diff --git a/arrow-select/src/interleave.rs b/arrow-select/src/interleave.rs
index 494fdb2931..3d8aaeccdc 100644
--- a/arrow-select/src/interleave.rs
+++ b/arrow-select/src/interleave.rs
@@ -211,7 +211,7 @@ fn interleave_primitive<T: ArrowPrimitiveType>(
}
// SAFETY: all `len` elements have been initialized
- debug_assert!(base == len);
+ debug_assert_eq!(base, len);
unsafe { output.set_len(len) };
let array = PrimitiveArray::<T>::try_new(output.into(),
interleaved.nulls)?;
diff --git a/arrow-select/src/merge.rs b/arrow-select/src/merge.rs
index 2198194380..be69054cbd 100644
--- a/arrow-select/src/merge.rs
+++ b/arrow-select/src/merge.rs
@@ -174,10 +174,9 @@ pub fn merge_n(values: &[&dyn Array], indices: &[impl
MergeIndex]) -> Result<Arr
if end_row_ix == indices.len() {
break;
- } else {
- // Set the start_row_ix for the next slice.
- start_row_ix = end_row_ix;
}
+ // Set the start_row_ix for the next slice.
+ start_row_ix = end_row_ix;
}
Ok(make_array(mutable.freeze()))
diff --git a/arrow-select/src/take.rs b/arrow-select/src/take.rs
index d429bbd268..55b3d521ab 100644
--- a/arrow-select/src/take.rs
+++ b/arrow-select/src/take.rs
@@ -172,12 +172,12 @@ where
T::Native: Display,
{
let Some(len) = T::Native::from_usize(len) else {
- if T::DATA_TYPE.is_integer() {
+ return if T::DATA_TYPE.is_integer() {
// the biggest representable value for T::Native is lower than
len, e.g: u8::MAX < 512, no need to check bounds
- return Ok(());
+ Ok(())
} else {
- return Err(ArrowError::ComputeError("Cast to usize
failed".to_string()));
- }
+ Err(ArrowError::ComputeError("Cast to usize failed".to_string()))
+ };
};
if indices.null_count() > 0 {
diff --git a/arrow-select/src/union_extract.rs
b/arrow-select/src/union_extract.rs
index b5a547ffd4..00fe6db734 100644
--- a/arrow-select/src/union_extract.rs
+++ b/arrow-select/src/union_extract.rs
@@ -354,9 +354,8 @@ fn eq_scalar_inner(chunk_size: usize, type_ids: &[i8],
target: i8) -> BoolValue
if false_bits == type_ids.len() {
return BoolValue::Scalar(false);
- } else {
- (false_bits, false)
}
+ (false_bits, false)
} else {
(true_bits, true)
};
diff --git a/arrow/src/util/test_util.rs b/arrow/src/util/test_util.rs
index 2a8bf05852..90b3b4e272 100644
--- a/arrow/src/util/test_util.rs
+++ b/arrow/src/util/test_util.rs
@@ -120,16 +120,16 @@ fn get_data_dir(udf_env: &str, submodule_data: &str) ->
Result<PathBuf, Box<dyn
let trimmed = dir.trim().to_string();
if !trimmed.is_empty() {
let pb = PathBuf::from(trimmed);
- if pb.is_dir() {
- return Ok(pb);
+ return if pb.is_dir() {
+ Ok(pb)
} else {
- return Err(format!(
+ Err(format!(
"the data dir `{}` defined by env {} not found",
pb.display(),
udf_env
)
- .into());
- }
+ .into())
+ };
}
}
diff --git a/parquet-geospatial/src/interval.rs
b/parquet-geospatial/src/interval.rs
index 3b3b9d868b..97a84ec00c 100644
--- a/parquet-geospatial/src/interval.rs
+++ b/parquet-geospatial/src/interval.rs
@@ -423,11 +423,11 @@ impl IntervalTrait for WraparoundInterval {
let new_right = right.merge_interval(&other_right);
// If the left and right intervals intersect each other, we
need the full interval
- if new_left.intersects_interval(&new_right) {
- return WraparoundInterval::full();
+ return if new_left.intersects_interval(&new_right) {
+ WraparoundInterval::full()
} else {
- return WraparoundInterval::new(new_right.lo(),
new_left.hi());
- }
+ WraparoundInterval::new(new_right.lo(), new_left.hi())
+ };
}
// Neither are wraparound: just merge the inner intervals
(false, false) => {
diff --git a/parquet-variant-compute/src/arrow_to_variant.rs
b/parquet-variant-compute/src/arrow_to_variant.rs
index 380946cc91..7bf80ad0b3 100644
--- a/parquet-variant-compute/src/arrow_to_variant.rs
+++ b/parquet-variant-compute/src/arrow_to_variant.rs
@@ -365,16 +365,16 @@ macro_rules! define_row_builder {
$(
// NOTE: The `?` macro expansion fails without the
type annotation.
let Some(value): Option<$option_ty> = value else {
- if !self.options.safe {
- return
Err(ArrowError::ComputeError(format!(
- "Failed to convert value at index
{index}: conversion failed",
- )));
- } else {
+ return if self.options.safe {
// Overflow is encoded as Variant::Null,
// distinct from None indicating a missing
value
builder.append_value(Variant::Null);
- return Ok(());
- }
+ Ok(())
+ } else {
+ Err(ArrowError::ComputeError(format!(
+ "Failed to convert value at index
{index}: conversion failed",
+ )))
+ };
};
)?
)?
diff --git a/parquet-variant/tests/variant_interop.rs
b/parquet-variant/tests/variant_interop.rs
index 229ea44937..9c8b21aa5f 100644
--- a/parquet-variant/tests/variant_interop.rs
+++ b/parquet-variant/tests/variant_interop.rs
@@ -54,11 +54,8 @@ fn cases_dir() -> PathBuf {
let pb = PathBuf::from(trimmed).join("..").join("variant");
if pb.is_dir() {
return pb;
- } else {
- panic!(
- "Can't find variant data at `{pb:?}`. Used value of env
`{env_name}`../variant ",
- )
}
+ panic!("Can't find variant data at `{pb:?}`. Used value of env
`{env_name}`../variant ")
}
}
diff --git a/parquet/src/arrow/arrow_reader/selection/selector.rs
b/parquet/src/arrow/arrow_reader/selection/selector.rs
index 680cc72879..291b0aaa84 100644
--- a/parquet/src/arrow/arrow_reader/selection/selector.rs
+++ b/parquet/src/arrow/arrow_reader/selection/selector.rs
@@ -136,9 +136,8 @@ pub(super) fn limit_selectors(
selection.row_count = limit;
selectors.truncate(idx + 1);
break;
- } else {
- limit -= selection.row_count;
}
+ limit -= selection.row_count;
}
}
selectors
diff --git a/parquet/src/arrow/arrow_writer/levels.rs
b/parquet/src/arrow/arrow_writer/levels.rs
index 40417ef73c..d62378bbde 100644
--- a/parquet/src/arrow/arrow_writer/levels.rs
+++ b/parquet/src/arrow/arrow_writer/levels.rs
@@ -418,7 +418,7 @@ impl LevelInfoBuilder {
// mapping between child elements and rep_level entries, the
position
// of each slot's first element is directly computable from
offsets.
child.visit_leaves(|leaf| {
- debug_assert!(leaf.max_rep_level == ctx.rep_level);
+ debug_assert_eq!(leaf.max_rep_level, ctx.rep_level);
let rep_levels = leaf.rep_levels.materialize_mut().unwrap();
let batch_len = values_end - values_start;
let batch_base = rep_levels.len() - batch_len;
diff --git a/parquet/src/arrow/buffer/dictionary_buffer.rs
b/parquet/src/arrow/buffer/dictionary_buffer.rs
index c2f92bf4c5..441da3a501 100644
--- a/parquet/src/arrow/buffer/dictionary_buffer.rs
+++ b/parquet/src/arrow/buffer/dictionary_buffer.rs
@@ -400,6 +400,10 @@ fn hash_byte_slices<I: ArrowNativeType>(offsets: &[I],
values: &[u8], scratch: &
let state = RandomState::new();
// SAFETY: MutableBuffer is 64-byte aligned; scratch is sized to exactly
count * size_of::<u64>()
+ #[expect(
+ clippy::cast_ptr_alignment,
+ reason = "MutableBuffer is 64-byte aligned"
+ )]
let hash_slots = unsafe {
from_raw_parts_mut(scratch.as_mut_ptr().cast::<u64>(), count) };
for idx in 0..count {
@@ -415,8 +419,14 @@ fn hash_byte_slices<I: ArrowNativeType>(offsets: &[I],
values: &[u8], scratch: &
#[inline]
fn hashes_as_u64(scratch: &[u8]) -> &[u64] {
let n = scratch.len() / size_of::<u64>();
- // SAFETY: scratch was written as u64s by hash_byte_slices
- unsafe { from_raw_parts(scratch.as_ptr().cast::<u64>(), n) }
+ // SAFETY: scratch was written as u64s by hash_byte_slices, and
MutableBuffer is 64-byte aligned
+ #[expect(
+ clippy::cast_ptr_alignment,
+ reason = "MutableBuffer is 64-byte aligned"
+ )]
+ unsafe {
+ from_raw_parts(scratch.as_ptr().cast::<u64>(), n)
+ }
}
#[cfg(test)]
diff --git a/parquet/src/arrow/in_memory_row_group.rs
b/parquet/src/arrow/in_memory_row_group.rs
index db523e5be1..9abf596022 100644
--- a/parquet/src/arrow/in_memory_row_group.rs
+++ b/parquet/src/arrow/in_memory_row_group.rs
@@ -84,10 +84,10 @@ impl InMemoryRowGroup<'_> {
// If the first page does not start at the beginning of
the column,
// then we need to also fetch a dictionary page.
let mut ranges: Vec<Range<u64>> = vec![];
- let (start, _len) = chunk_meta.byte_range();
+ let (start, len) = chunk_meta.byte_range();
let Some(offset_idx) = offset_index[idx].as_ref() else {
// No offset index for this column, fetch the entire
column
- ranges.push(start..start + _len);
+ ranges.push(start..start + len);
return ranges;
};
diff --git a/parquet/src/column/writer/mod.rs b/parquet/src/column/writer/mod.rs
index a91d2af53d..1f4466c082 100644
--- a/parquet/src/column/writer/mod.rs
+++ b/parquet/src/column/writer/mod.rs
@@ -4430,12 +4430,12 @@ mod tests {
assert_eq!(1, r.rows_written);
let stats = r.metadata.statistics().expect("statistics");
- if let Statistics::ByteArray(_stats) = stats {
- let min_value = _stats.min_opt().unwrap();
- let max_value = _stats.max_opt().unwrap();
+ if let Statistics::ByteArray(stats) = stats {
+ let min_value = stats.min_opt().unwrap();
+ let max_value = stats.max_opt().unwrap();
- assert!(!_stats.min_is_exact());
- assert!(!_stats.max_is_exact());
+ assert!(!stats.min_is_exact());
+ assert!(!stats.max_is_exact());
let expected_len = 64;
assert_eq!(min_value.len(), expected_len);
@@ -4480,12 +4480,12 @@ mod tests {
let stats = r.metadata.statistics().expect("statistics");
assert_eq!(stats.null_count_opt(), Some(0));
assert_eq!(stats.distinct_count_opt(), None);
- if let Statistics::ByteArray(_stats) = stats {
- let min_value = _stats.min_opt().unwrap();
- let max_value = _stats.max_opt().unwrap();
+ if let Statistics::ByteArray(stats) = stats {
+ let min_value = stats.min_opt().unwrap();
+ let max_value = stats.max_opt().unwrap();
- assert!(!_stats.min_is_exact());
- assert!(!_stats.max_is_exact());
+ assert!(!stats.min_is_exact());
+ assert!(!stats.max_is_exact());
assert_eq!(min_value.len(), TEST_TRUNCATE_LENGTH);
assert_eq!(max_value.len(), TEST_TRUNCATE_LENGTH);
@@ -4532,12 +4532,12 @@ mod tests {
let stats = r.metadata.statistics().expect("statistics");
assert_eq!(stats.null_count_opt(), Some(0));
assert_eq!(stats.distinct_count_opt(), None);
- if let Statistics::FixedLenByteArray(_stats) = stats {
- let min_value = _stats.min_opt().unwrap();
- let max_value = _stats.max_opt().unwrap();
+ if let Statistics::FixedLenByteArray(stats) = stats {
+ let min_value = stats.min_opt().unwrap();
+ let max_value = stats.max_opt().unwrap();
- assert!(!_stats.min_is_exact());
- assert!(!_stats.max_is_exact());
+ assert!(!stats.min_is_exact());
+ assert!(!stats.max_is_exact());
assert_eq!(min_value.len(), TEST_TRUNCATE_LENGTH);
assert_eq!(max_value.len(), TEST_TRUNCATE_LENGTH);
diff --git a/parquet/src/compression.rs b/parquet/src/compression.rs
index 74d740348a..2eb71f621c 100644
--- a/parquet/src/compression.rs
+++ b/parquet/src/compression.rs
@@ -146,6 +146,13 @@ pub(crate) trait CompressionLevel<T: std::fmt::Display +
std::cmp::PartialOrd> {
/// Given the compression type `codec`, returns a codec used to compress and
decompress
/// bytes for the compression type.
/// This returns `None` if the codec type is `UNCOMPRESSED`.
+#[cfg_attr(
+ any(feature = "lz4", test),
+ expect(
+ clippy::used_underscore_binding,
+ reason = "`_options` is only read when the `lz4` feature is on"
+ )
+)]
pub fn create_codec(codec: CodecType, _options: &CodecOptions) ->
Result<Option<Box<dyn Codec>>> {
#[cfg_attr(
any(
diff --git a/parquet/src/data_type.rs b/parquet/src/data_type.rs
index 29f5f07fc6..432cf28d81 100644
--- a/parquet/src/data_type.rs
+++ b/parquet/src/data_type.rs
@@ -1407,11 +1407,11 @@ mod tests {
Decimal::from_i32(3, 5, 2)
);
- assert!(Decimal::from_i32(222, 5, 2) != Decimal::from_i32(111, 5, 2));
- assert!(Decimal::from_i32(222, 5, 2) != Decimal::from_i32(222, 6, 2));
- assert!(Decimal::from_i32(222, 5, 2) != Decimal::from_i32(222, 5, 3));
+ assert_ne!(Decimal::from_i32(222, 5, 2), Decimal::from_i32(111, 5, 2));
+ assert_ne!(Decimal::from_i32(222, 5, 2), Decimal::from_i32(222, 6, 2));
+ assert_ne!(Decimal::from_i32(222, 5, 2), Decimal::from_i32(222, 5, 3));
- assert!(Decimal::from_i64(222, 5, 2) != Decimal::from_i32(222, 5, 2));
+ assert_ne!(Decimal::from_i64(222, 5, 2), Decimal::from_i32(222, 5, 2));
}
#[test]
diff --git a/parquet/src/file/metadata/reader.rs
b/parquet/src/file/metadata/reader.rs
index 018cf440cd..9589e308d7 100644
--- a/parquet/src/file/metadata/reader.rs
+++ b/parquet/src/file/metadata/reader.rs
@@ -290,16 +290,16 @@ impl ParquetMetaDataReader {
Err(ParquetError::NeedMoreData(needed)) => {
// If reader is the same length as `file_size` then presumably
there is no more to
// read, so return an EOF error.
- if file_size == reader.len() || needed as u64 > file_size {
- return Err(eof_err!(
+ return if file_size == reader.len() || needed as u64 >
file_size {
+ Err(eof_err!(
"Parquet file too small. Size is {} but need {}",
file_size,
needed
- ));
+ ))
} else {
// Ask for a larger buffer
- return Err(ParquetError::NeedMoreData(needed));
- }
+ Err(ParquetError::NeedMoreData(needed))
+ };
}
Err(e) => return Err(e),
};
@@ -355,16 +355,16 @@ impl ParquetMetaDataReader {
let file_range = file_size.saturating_sub(reader.len())..file_size;
if !(file_range.contains(&range.start) &&
file_range.contains(&range.end)) {
// Requested range starts beyond EOF
- if range.end > file_size {
- return Err(eof_err!(
+ return if range.end > file_size {
+ Err(eof_err!(
"Parquet file too small. Range {range:?} is beyond file
bounds {file_size}",
- ));
+ ))
} else {
// Ask for a larger buffer
- return Err(ParquetError::NeedMoreData(
+ Err(ParquetError::NeedMoreData(
(file_size - range.start).try_into()?,
- ));
- }
+ ))
+ };
}
// Perform extra sanity check to make sure `range` and the footer
metadata don't
diff --git a/parquet/src/file/statistics.rs b/parquet/src/file/statistics.rs
index 5c9304652c..0c6503a5a9 100644
--- a/parquet/src/file/statistics.rs
+++ b/parquet/src/file/statistics.rs
@@ -867,30 +867,46 @@ mod tests {
fn test_statistics_partial_eq() {
let expected = Statistics::int32(Some(12), Some(45), None, Some(11),
true);
- assert!(Statistics::int32(Some(12), Some(45), None, Some(11), true) ==
expected);
- assert!(Statistics::int32(Some(11), Some(45), None, Some(11), true) !=
expected);
- assert!(Statistics::int32(Some(12), Some(44), None, Some(11), true) !=
expected);
- assert!(Statistics::int32(Some(12), Some(45), None, Some(23), true) !=
expected);
- assert!(Statistics::int32(Some(12), Some(45), None, Some(11), false)
!= expected);
-
- assert!(
- Statistics::int32(Some(12), Some(45), None, Some(11), false)
- != Statistics::int64(Some(12), Some(45), None, Some(11), false)
+ assert_eq!(
+ Statistics::int32(Some(12), Some(45), None, Some(11), true),
+ expected
+ );
+ assert_ne!(
+ Statistics::int32(Some(11), Some(45), None, Some(11), true),
+ expected
+ );
+ assert_ne!(
+ Statistics::int32(Some(12), Some(44), None, Some(11), true),
+ expected
+ );
+ assert_ne!(
+ Statistics::int32(Some(12), Some(45), None, Some(23), true),
+ expected
+ );
+ assert_ne!(
+ Statistics::int32(Some(12), Some(45), None, Some(11), false),
+ expected
+ );
+
+ assert_ne!(
+ Statistics::int32(Some(12), Some(45), None, Some(11), false),
+ Statistics::int64(Some(12), Some(45), None, Some(11), false)
);
- assert!(
- Statistics::boolean(Some(false), Some(true), None, None, true)
- != Statistics::double(Some(1.2), Some(4.5), None, None, true)
+ assert_ne!(
+ Statistics::boolean(Some(false), Some(true), None, None, true),
+ Statistics::double(Some(1.2), Some(4.5), None, None, true)
);
- assert!(
+ assert_ne!(
Statistics::byte_array(
Some(ByteArray::from(vec![1, 2, 3])),
Some(ByteArray::from(vec![1, 2, 3])),
None,
None,
true
- ) != Statistics::fixed_len_byte_array(
+ ),
+ Statistics::fixed_len_byte_array(
Some(ByteArray::from(vec![1, 2, 3]).into()),
Some(ByteArray::from(vec![1, 2, 3]).into()),
None,
@@ -899,14 +915,15 @@ mod tests {
)
);
- assert!(
+ assert_ne!(
Statistics::byte_array(
Some(ByteArray::from(vec![1, 2, 3])),
Some(ByteArray::from(vec![1, 2, 3])),
None,
None,
true,
- ) != Statistics::ByteArray(
+ ),
+ Statistics::ByteArray(
ValueStatistics::new(
Some(ByteArray::from(vec![1, 2, 3])),
Some(ByteArray::from(vec![1, 2, 3])),
@@ -918,14 +935,15 @@ mod tests {
)
);
- assert!(
+ assert_ne!(
Statistics::fixed_len_byte_array(
Some(FixedLenByteArray::from(vec![1, 2, 3])),
Some(FixedLenByteArray::from(vec![1, 2, 3])),
None,
None,
true,
- ) != Statistics::FixedLenByteArray(
+ ),
+ Statistics::FixedLenByteArray(
ValueStatistics::new(
Some(FixedLenByteArray::from(vec![1, 2, 3])),
Some(FixedLenByteArray::from(vec![1, 2, 3])),
diff --git a/parquet/src/geospatial/accumulator.rs
b/parquet/src/geospatial/accumulator.rs
index 477721897d..adc96e9ced 100644
--- a/parquet/src/geospatial/accumulator.rs
+++ b/parquet/src/geospatial/accumulator.rs
@@ -116,6 +116,13 @@ pub trait GeoStatsAccumulator: Send {
pub struct DefaultGeoStatsAccumulatorFactory {}
impl GeoStatsAccumulatorFactory for DefaultGeoStatsAccumulatorFactory {
+ #[cfg_attr(
+ feature = "geospatial",
+ expect(
+ clippy::used_underscore_binding,
+ reason = "`_descr` is only read when the `geospatial` feature is
on"
+ )
+ )]
fn new_accumulator(&self, _descr: &ColumnDescPtr) -> Box<dyn
GeoStatsAccumulator> {
#[cfg(feature = "geospatial")]
if let Some(crate::basic::LogicalType::Geometry { .. }) =
_descr.logical_type_ref() {
diff --git a/parquet/src/schema/parser.rs b/parquet/src/schema/parser.rs
index 204246b5db..9ac7a62273 100644
--- a/parquet/src/schema/parser.rs
+++ b/parquet/src/schema/parser.rs
@@ -209,10 +209,9 @@ impl Parser<'_> {
while let Some(value) = self.tokenizer.next() {
if value == "}" {
break;
- } else {
- self.tokenizer.backtrack();
- vec.push(Arc::new(self.add_type()?));
}
+ self.tokenizer.backtrack();
+ vec.push(Arc::new(self.add_type()?));
}
Ok(vec)
}
diff --git a/parquet/src/util/bit_util.rs b/parquet/src/util/bit_util.rs
index 20ce146463..ab1524f639 100644
--- a/parquet/src/util/bit_util.rs
+++ b/parquet/src/util/bit_util.rs
@@ -175,7 +175,7 @@ impl FromBitpacked for bool {
#[inline]
fn unpack_batch(input: &[u8], output: &mut [Self], num_bits: usize) {
- assert!(num_bits == 1);
+ assert_eq!(num_bits, 1);
// Safety:
// we asserted that we will only decode with a bitwidth of 1,
// so the u8 can only be 0 or 1, which are the valid representations
of a bool.
diff --git a/parquet/tests/encryption/encryption_async.rs
b/parquet/tests/encryption/encryption_async.rs
index 35b9878a68..189eaccb8b 100644
--- a/parquet/tests/encryption/encryption_async.rs
+++ b/parquet/tests/encryption/encryption_async.rs
@@ -800,7 +800,7 @@ fn spawn_parquet_parallel_serialization_task(
let mut current_rg_rows = 0;
while let Some(mut rb) = data.recv().await {
- // This loop allows the "else" block to repeatedly split the
RecordBatch to handle the case
+ // This loop repeatedly splits the RecordBatch to handle the case
// when max_row_group_rows < execution.batch_size as an
alternative to a recursive async
// function.
loop {
@@ -808,33 +808,33 @@ fn spawn_parquet_parallel_serialization_task(
send_arrays_to_column_writers(&col_array_channels, &rb,
&schema).await?;
current_rg_rows += rb.num_rows();
break;
- } else {
- let rows_left = max_row_group_rows - current_rg_rows;
- let rb_split = rb.slice(0, rows_left);
- send_arrays_to_column_writers(&col_array_channels,
&rb_split, &schema).await?;
-
- // Signal the parallel column writers that the RowGroup is
done, join and finalize RowGroup
- // on a separate task, so that we can immediately start on
the next RG before waiting
- // for the current one to finish.
- drop(col_array_channels);
-
- let finalize_rg_task =
- spawn_rg_join_and_finalize_task(col_writer_tasks,
max_row_group_rows);
-
- // Do not surface error from closed channel (means
something
- // else hit an error, and the plan is shutting down).
- if serialize_tx.send(finalize_rg_task).await.is_err() {
- return Ok(());
- }
-
- current_rg_rows = 0;
- rb = rb.slice(rows_left, rb.num_rows() - rows_left);
-
- row_group_index += 1;
- let column_writers =
writer_factory.create_column_writers(row_group_index)?;
- (col_writer_tasks, col_array_channels) =
- spawn_column_parallel_row_group_writer(column_writers,
100)?;
}
+
+ let rows_left = max_row_group_rows - current_rg_rows;
+ let rb_split = rb.slice(0, rows_left);
+ send_arrays_to_column_writers(&col_array_channels, &rb_split,
&schema).await?;
+
+ // Signal the parallel column writers that the RowGroup is
done, join and finalize RowGroup
+ // on a separate task, so that we can immediately start on the
next RG before waiting
+ // for the current one to finish.
+ drop(col_array_channels);
+
+ let finalize_rg_task =
+ spawn_rg_join_and_finalize_task(col_writer_tasks,
max_row_group_rows);
+
+ // Do not surface error from closed channel (means something
+ // else hit an error, and the plan is shutting down).
+ if serialize_tx.send(finalize_rg_task).await.is_err() {
+ return Ok(());
+ }
+
+ current_rg_rows = 0;
+ rb = rb.slice(rows_left, rb.num_rows() - rows_left);
+
+ row_group_index += 1;
+ let column_writers =
writer_factory.create_column_writers(row_group_index)?;
+ (col_writer_tasks, col_array_channels) =
+ spawn_column_parallel_row_group_writer(column_writers,
100)?;
}
}
