Add interval mul/div support for Float64 by peterxcli · Pull Request #10409 · apache/arrow-rs

peterxcli · 2026-07-22T07:40:38Z

Which issue does this PR close?

Closes Support multiple and divide on intervals #6335.
Closes Support interval mul / div double arithmetic #9030.

Rationale for this change

#10336 added interval multiplication by Int64. Floating-point multiplication and division require preserving calendar components rather than flattening the entire interval into a duration.

This PR adapts DuckDB's INTERVAL * DOUBLE behavior. Whole months and days remain calendar components, while fractional months cascade to days using 30 days per month and fractional days cascade to nanoseconds using 24 hours per day.

DuckDB explicitly attributes its implementation to PostgreSQL's interval_mul:

What changes are included in this PR?

Add checked arithmetic for:

Interval(MonthDayNano) * Float64 (communicative)
Interval(MonthDayNano) / Float64
Other interval type or the factor type are expected to be coerced by the caller.

All results use Interval(MonthDayNano).

Are these changes tested?

reuses the checked integer path for integral Float64 factors, preserving exact nanoseconds
implements division by multiplying by the reciprocal, following DuckDB
uses ties-to-even rounding at nanosecond precision
checks division by zero and component overflow
preserves array, scalar, and null behavior

Interval / Int64, Float64 / Interval, wrapping multiplication, and additional numeric factor types are not added.

Are these changes tested?

Yes. Tests cover all three interval units, both multiplication operand orders, division, array and scalar operands, null propagation, component cascading, integral-factor precision, negative values, ties-to-even rounding, non-finite values, division by zero, and overflow.

cargo fmt --all -- --check
cargo test -p arrow-arith
cargo clippy -p arrow-arith --all-targets --all-features -- -D warnings

Are there any user-facing changes?

Yes. The checked mul and div kernels now accept the combinations listed above. Floating-point interval arithmetic always returns Interval(MonthDayNano).
There are no public API signature changes.

peterxcli · 2026-07-22T13:22:59Z

cc @milevin @alamb @tustvold @felipecrv @bkietz @findepi @Jefffrey would appreciate a look whenever you have time, thanks! Tagging you since you reviewed #6906 and have context here. Sorry for the ping!

alamb · 2026-07-23T20:59:07Z

Thanks - I'll try and find time for this but I need to focus on performance and bugfixes first

peterxcli · 2026-07-24T18:51:16Z

I need to focus on performance and bugfixes first

TIA. If you need any help on other "performance and bugfixes" issue, please let me know, I would like to take a stab at them 😄

alamb · 2026-07-24T21:25:01Z

I need to focus on performance and bugfixes first

TIA. If you need any help on other "performance and bugfixes" issue, please let me know, I would like to take a stab at them 😄

We are always looking for help reviewing other PRs (ensure test coverage, make sure you can unerstand the change, see if you can find any logic holes, etc) 🙏

Jefffrey · 2026-07-27T00:01:07Z

+        factor_scalar,
+        {
+            // Float scaling can create components that narrower interval units cannot store.
+            let interval = T::to_month_day_nano(interval);


i dont think we should have this conversion logic in arrow-rs; we should probably support only IntervalMonthDay explicitly and expect callers (e.g. datafusion) to coerce to this type themselves if they want to use this kernel. i think that has been the precedent for other kernels in arrow-rs

Thanks. I agree with caller-side coercion for the numeric operand: callers can coerce multiplication and division factors to Int64 or Float64, so arrow-arith does not need to support every numeric type.

I’m less clear that the same precedent requires coercing the interval representation. #10336 directly supports Interval(YearMonth), Interval(DayTime), and Interval(MonthDayNano) for multiplication by Int64; this PR follows the same input coverage. For Float64, the result must be MonthDayNano because fractional months and days can cascade into smaller components, and widening YearMonth/DayTime to MonthDayNano is lossless.

If callers should also normalize the interval representation, should the same rule apply to #10336, or is the distinction that integer multiplication preserves the input representation while floating-point arithmetic widens it?

Could you also point me to a specific kernel or PR establishing this precedent for coercion between related Arrow types? The precedent discussed in #6906 appears to concern heterogeneous numeric operands; limiting factors to Int64 or Float64 already follows that boundary.

since i64 multiplication preserves input type, its not doing any coercion so its fine as is. f64 requires converting to monthdaynano so this is something the caller should cast to instead of us casting inside an arithmetic kernel

Could you also point me to a specific kernel or PR establishing this precedent for coercion between related Arrow types? The precedent discussed in #6906 (comment) appears to concern heterogeneous numeric operands; limiting factors to Int64 or Float64 already follows that boundary.

the precedent is just the existing codebase as a whole; im not aware of any arithmetic kernels that cast the input types (apart from things like decimal widening, etc.) though i may be wrong

just in general, this type of input casting is best left to the caller to opt in to, and at the arrow-rs level its simpler to just support the exact types we actually support (in this cast f64 * monthdaynano)

since i64 multiplication preserves input type, its not doing any coercion so its fine as is. f64 requires converting to monthdaynano so this is something the caller should cast to instead of us casting inside an arithmetic kernel
just in general, this type of input casting is best left to the caller to opt in to, and at the arrow-rs level its simpler to just support the exact types we actually support (in this cast f64 * monthdaynano)

sounds fair, let me remove the conversion and only keep the Interval(MonthDayNano) * f64 input. Thanks for the explaination!

Jefffrey

could we keep the PR body up to date as well; it still refers to supporting yearmonth/daytime

Jefffrey · 2026-07-30T03:55:52Z

    ))
 }

+/// Multiplies using `IntervalMonthDayNano` as the common interval representation,


this docstring is out of date; no need to mention "common interval representation" since its only defined for monthdaynano intervals

Jefffrey · 2026-07-30T03:58:57Z

+                // DuckDB defines interval division as multiplication by the reciprocal:
+                // https://github.com/duckdb/duckdb/blob/21aca0424f1faf78b593b1e6fbfdd4846624c987/src/function/scalar/operator/arithmetic.cpp#L1102-L1110
+                Op::Div => interval_mul_f64(interval, 1. / factor),
+                _ => unreachable!(),


this unreachable!() seems reachable in normal usage (if we use add(mdn, f64) for example)

changed to throw exception!

Jefffrey · 2026-07-30T04:04:00Z

+    if !months_product.is_finite()
+        || months_product < f64::from(i32::MIN)
+        || months_product > f64::from(i32::MAX)
+    {
+        return Err(overflow("months"));
+    }
+    let months = months_product.to_i32().ok_or_else(|| overflow("months"))?;


does to_i32() not check the range and non-finite numbers when converting? trying to understand why we have two checks here

If only relying on to_i32(), i32::MIN * 1.000_000_000_4 truncates to valid i32::MIN, so it no longer overflow.

arrow-rs/arrow-arith/src/numeric.rs

Lines 1931 to 1940 in 8393e14

let factor = Float64Array::new_scalar(1.000_000_000_4);

let interval = IntervalMonthDayNanoArray::new_scalar(IntervalMonthDayNanoType::make_value(

i32::MIN,

0,

0,

));

assert!(matches!(

mul(&interval, &factor),

Err(ArrowError::ArithmeticOverflow(_))

));

Jefffrey · 2026-07-30T04:05:10Z

+    }
+    let mut days = days_product.to_i32().ok_or_else(|| overflow("days"))?;
+
+    let month_remainder = timestamp_round((months_product - f64::from(months)) * DAYS_PER_MONTH);


could we use f64::fract()` here instead?

good catch, makes it cleaner!

peterxcli · 2026-07-30T10:38:49Z

could we keep the PR body up to date as well; it still refers to supporting yearmonth/daytime

Thanks for the heads-up!

Add interval mul/div support for Float64

a4a98b7

Jefffrey reviewed Jul 27, 2026

View reviewed changes

review

7d6c897

github-actions Bot added the arrow Changes to the arrow crate label Jul 27, 2026

peterxcli added 3 commits July 27, 2026 20:50

commutative

1e21845

Restrict Float64 arithmetic to month-day-nano intervals

bc0773d

Merge branch 'main' into feat/interval-mul-div-with-f64

80bcb80

peterxcli requested a review from Jefffrey July 28, 2026 04:57

Merge branch 'main' into feat/interval-mul-div-with-f64

82a07a0

Jefffrey reviewed Jul 30, 2026

View reviewed changes

review

8393e14

	let factor = Float64Array::new_scalar(1.000_000_000_4);
	let interval = IntervalMonthDayNanoArray::new_scalar(IntervalMonthDayNanoType::make_value(
	i32::MIN,
	0,
	0,
	));
	assert!(matches!(
	mul(&interval, &factor),
	Err(ArrowError::ArithmeticOverflow(_))
	));

Uh oh!

Conversation

peterxcli commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

peterxcli commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alamb commented Jul 23, 2026

Uh oh!

peterxcli commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alamb commented Jul 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterxcli Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterxcli commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

peterxcli commented Jul 22, 2026 •

edited

Loading

peterxcli commented Jul 22, 2026 •

edited

Loading

peterxcli commented Jul 24, 2026 •

edited

Loading

peterxcli Jul 30, 2026 •

edited

Loading