This is an automated email from the ASF dual-hosted git repository.
alamb 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 aa09633380 fix: switch generic usages of `i128` to
`IntervalMonthDayNano` for MonthDayNano type (#10171)
aa09633380 is described below
commit aa096333804f47e363250ce42fe170e887d278ca
Author: Jeffrey Vo <[email protected]>
AuthorDate: Tue Jun 23 03:09:14 2026 +0900
fix: switch generic usages of `i128` to `IntervalMonthDayNano` for
MonthDayNano type (#10171)
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax.
-->
- Closes #5917
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->
Some functions were still using `i128` for generic type in relation to
monthdaynano type, which is incorrect since `i128` has an alignment of
16 whilst actual `IntervalMonthDayNano` type only has 8; this mismatch
can cause panics on certain code paths.
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->
Switch usage of `i128` to `IntervalMonthDayNano`
# Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
If this PR claims a performance improvement, please include evidence
such as benchmark results.
-->
Test from original issue.
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
If there are any breaking changes to public APIs, please call them out.
-->
No.
---
arrow-data/src/equal/mod.rs | 4 ++--
arrow-data/src/transform/mod.rs | 10 +++++++---
arrow/tests/array_transform.rs | 22 +++++++++++++++++++++-
3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/arrow-data/src/equal/mod.rs b/arrow-data/src/equal/mod.rs
index 7a310b1240..b241b8cf66 100644
--- a/arrow-data/src/equal/mod.rs
+++ b/arrow-data/src/equal/mod.rs
@@ -20,7 +20,7 @@
//! depend on dynamic casting of `Array`.
use crate::data::ArrayData;
-use arrow_buffer::i256;
+use arrow_buffer::{IntervalMonthDayNano, i256};
use arrow_schema::{DataType, IntervalUnit};
use half::f16;
@@ -92,7 +92,7 @@ fn equal_values(
| DataType::Timestamp(_, _)
| DataType::Duration(_) => primitive_equal::<i64>(lhs, rhs, lhs_start,
rhs_start, len),
DataType::Interval(IntervalUnit::MonthDayNano) => {
- primitive_equal::<i128>(lhs, rhs, lhs_start, rhs_start, len)
+ primitive_equal::<IntervalMonthDayNano>(lhs, rhs, lhs_start,
rhs_start, len)
}
DataType::Utf8 | DataType::Binary => {
variable_sized_equal::<i32>(lhs, rhs, lhs_start, rhs_start, len)
diff --git a/arrow-data/src/transform/mod.rs b/arrow-data/src/transform/mod.rs
index 66f6603f02..df9a22b839 100644
--- a/arrow-data/src/transform/mod.rs
+++ b/arrow-data/src/transform/mod.rs
@@ -23,7 +23,7 @@
use super::{ArrayData, ArrayDataBuilder, ByteView, data::new_buffers};
use crate::bit_mask::set_bits;
use arrow_buffer::buffer::{BooleanBuffer, NullBuffer};
-use arrow_buffer::{ArrowNativeType, Buffer, MutableBuffer, bit_util, i256};
+use arrow_buffer::{ArrowNativeType, Buffer, IntervalMonthDayNano,
MutableBuffer, bit_util, i256};
use arrow_schema::{ArrowError, DataType, IntervalUnit, UnionMode};
use half::f16;
use num_integer::Integer;
@@ -257,7 +257,9 @@ fn build_extend(array: &ArrayData) -> Extend<'_> {
| DataType::Timestamp(_, _)
| DataType::Duration(_)
| DataType::Interval(IntervalUnit::DayTime) =>
primitive::build_extend::<i64>(array),
- DataType::Interval(IntervalUnit::MonthDayNano) =>
primitive::build_extend::<i128>(array),
+ DataType::Interval(IntervalUnit::MonthDayNano) => {
+ primitive::build_extend::<IntervalMonthDayNano>(array)
+ }
DataType::Decimal32(_, _) => primitive::build_extend::<i32>(array),
DataType::Decimal64(_, _) => primitive::build_extend::<i64>(array),
DataType::Decimal128(_, _) => primitive::build_extend::<i128>(array),
@@ -304,7 +306,9 @@ fn build_extend_nulls(data_type: &DataType) -> ExtendNulls {
| DataType::Timestamp(_, _)
| DataType::Duration(_)
| DataType::Interval(IntervalUnit::DayTime) =>
primitive::extend_nulls::<i64>,
- DataType::Interval(IntervalUnit::MonthDayNano) =>
primitive::extend_nulls::<i128>,
+ DataType::Interval(IntervalUnit::MonthDayNano) => {
+ primitive::extend_nulls::<IntervalMonthDayNano>
+ }
DataType::Decimal32(_, _) => primitive::extend_nulls::<i32>,
DataType::Decimal64(_, _) => primitive::extend_nulls::<i64>,
DataType::Decimal128(_, _) => primitive::extend_nulls::<i128>,
diff --git a/arrow/tests/array_transform.rs b/arrow/tests/array_transform.rs
index c24d0992a4..0dd4dd5f5e 100644
--- a/arrow/tests/array_transform.rs
+++ b/arrow/tests/array_transform.rs
@@ -21,7 +21,7 @@ use arrow::array::{
MapBuilder, NullArray, StringArray, StringBuilder,
StringDictionaryBuilder, StructArray,
UInt8Array, UInt16Array, UInt16Builder, UnionArray,
};
-use arrow::datatypes::Int16Type;
+use arrow::datatypes::{Int16Type, IntervalMonthDayNanoType};
use arrow_array::StringViewArray;
use arrow_buffer::{Buffer, ScalarBuffer};
use arrow_data::ArrayData;
@@ -1239,3 +1239,23 @@ fn test_extend_nulls_dense_union() {
// Second child (str) should have 1 entry from extend
assert_eq!(result_array.child(1).len(), 1);
}
+
+#[test]
+fn test_interval_month_day_nano_aligned_to_8() {
+ let ty = DataType::Interval(arrow_schema::IntervalUnit::MonthDayNano);
+
+ let offset = 8;
+ let mut buffer = arrow_buffer::MutableBuffer::new(0);
+ buffer.extend_zeros(offset);
+ buffer.push(IntervalMonthDayNanoType::make_value(0, 0, 0));
+ let buffer = Buffer::from(buffer).slice(offset);
+
+ let b = ArrayData::builder(ty.clone())
+ .add_buffer(buffer)
+ .align_buffers(true) // no alignment should be done
+ .build()
+ .unwrap();
+
+ // roundtripping through arraydata shouldnt panic
+ MutableArrayData::new(vec![&b], false, 0);
+}
