You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/10/23 19:49:07 UTC
(datafusion) branch gh-readonly-queue/main/pr-26676-c6869995c11c0e6661106f9d188dede8672ecc69 created (now 702803d022)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-26676-c6869995c11c0e6661106f9d188dede8672ecc69
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 702803d022 fix: keep fractional seconds in EXTRACT(EPOCH) for Duration (#26676)
This branch includes the following new commits:
new 702803d022 fix: keep fractional seconds in EXTRACT(EPOCH) for Duration (#26676)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: fix: keep fractional seconds in EXTRACT(EPOCH) for Duration (#26676)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-26676-c6869995c11c0e6661106f9d188dede8672ecc69
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 702803d0220cf444da5cf8c8d4ada8393ec89d54
Author: Matt Butrovich <mb...@users.noreply.github.com>
AuthorDate: Fri Oct 23 19:48:41 2026 +0000
fix: keep fractional seconds in EXTRACT(EPOCH) for Duration (#26676)
## Which issue does this PR close?
- Closes #26673.
## Rationale for this change
The old code was complicated because it reused a helper written for
timestamps. #12514 sent `Duration` epochs through
[`seconds`](https://github.com/apache/datafusion/blob/c76ed5396ad3f4a4dc9d9ceeddc419a146b871f9/datafusion/functions/src/datetime/date_part.rs#L507-L547)
and added `% 1_000_000_000` and a NULL-as-zero branch to recover the
fraction, which works only while the total fits in `i32`. #19807 fixed
the same bug for `Interval` and left `Duration` on that path.
## What changes are included in this PR?
The `Duration` arms of `epoch` now divide the `i64` value by the unit's
factor with `unary`, like the `Timestamp` arms, and `seconds` is gone.
The first commit adds two cases to the `datetime_expressions` bench
without the fix, so the benchmark bot can compare against it.
It is also 2.5x to 12x faster on the benchmark bot. `main` made three
passes over the batch and fell back to a row-by-row iterator on any NULL
or large value. The new loop takes the same time whatever the values or
NULLs.
<img width="1600" height="868" alt="epoch-duration-bot"
src="https://github.com/user-attachments/assets/c70a3f6e-81d6-40ba-ab2d-0cbd832643fc"
/>
<details><summary>Numbers and codegen</summary>
Speedups over `353e4cee5` (the bench commit) on 1000 rows, with the
bot's thin LTO. "Nulls" means 10% of rows are NULL. The aligned runs add
`-align-all-functions=6 -align-all-nofallthru-blocks=6`.
| Case |
[Neoverse-V2](https://github.com/apache/datafusion/pull/26676#issuecomment-6372747404)
|
[aligned](https://github.com/apache/datafusion/pull/26676#issuecomment-6372830987)
| [Xeon
8581C](https://github.com/apache/datafusion/pull/26676#issuecomment-6372833894)
|
[aligned](https://github.com/apache/datafusion/pull/26676#issuecomment-6372840740)
|
|---|---|---|---|---|
| `duration_ns` | 5.60x | 5.57x | 8.09x | 6.95x |
| `duration_ns_nulls` | 7.11x | 7.31x | 12.08x | 10.68x |
| `duration_ns_small` | 2.50x | 2.45x | 4.50x | 3.53x |
| `ns` (unchanged) | 1.00x | 1.01x | 1.09x | 1.03x |
In the default Xeon run the unchanged cases are 1.09x to 1.19x, so that
run overstates the gain by about 15%.
Timing each arrow kernel of `main`'s path separately on 8192 rows on an
Apple M5 Max accounts for its totals.
| Values | `date_part(Second)` | `date_part(Nanosecond)` | Combine | Sum
|
|---|---|---|---|---|
| Under 2.1 s, `binary` | 2.94 us | 2.32 us | 2.44 us | 7.70 us |
| Large, iterator `collect` | 2.92 us | 12.86 us | 16.47 us | 32.25 us |
LLVM merges the new loops with the existing `Timestamp` and `Time64`
ones, so this PR adds no new loop code. On aarch64 the loop handles 8
rows per iteration with `scvtf.2d` and `fdiv.2d`. On x86_64 the division
is packed (`divpd`, `vdivpd` with AVX2) and the `i64` to `f64`
conversion is scalar `cvtsi2sd`, because the packed form needs
AVX-512DQ. I read this with `cargo asm` (rustc 1.99.0,
`release-nonlto`).
</details>
The code divides rather than multiplying by `1e-9`. The division is most
of the loop's time on Neoverse-V2, where `date_part_epoch_s_1000` (no
division) takes 412 ns against 1.21 us for `ns`. The multiply would be
faster, but it changes 14% of `Duration(ms)` and 41% of `Duration(ns)`
results, for example 12.92335 s becomes `12.923350000000001`.
## What is the testing strategy for this PR?
New cases in `datafusion/sqllogictest/test_files/datetime/date_part.slt`
under `test_extract_duration` cover the `i32::MAX` ns boundary, each
`Duration` unit, the issue's reproducer, negative values, NULLs mixed
with values, and `i64` extremes.
## Are there any user-facing changes?
`EXTRACT(EPOCH)` on `Duration` returns correct results. There are no
public API changes.
## AI Disclosure
I wrote this PR with help from Claude but understand and support the
core approach, implementation, and test coverage.
---
.../benches/datetime_expressions/date_part.rs | 32 ++++++++++
datafusion/functions/src/datetime/date_part.rs | 57 ++++-------------
.../sqllogictest/test_files/datetime/date_part.slt | 72 ++++++++++++++++++++++
3 files changed, 117 insertions(+), 44 deletions(-)
diff --git a/datafusion/functions/benches/datetime_expressions/date_part.rs b/datafusion/functions/benches/datetime_expressions/date_part.rs
index aa4b3e1dba..219f75f45e 100644
--- a/datafusion/functions/benches/datetime_expressions/date_part.rs
+++ b/datafusion/functions/benches/datetime_expressions/date_part.rs
@@ -39,6 +39,7 @@ const BATCH_SIZE: usize = 1000;
const TS_BOUND: i64 = 2_006_463_600;
const SEC_DAY: i64 = 86_400;
const DAYS_SINCE_EPOCH: i64 = TS_BOUND / SEC_DAY;
+const NULL_DENSITY: f64 = 0.1;
fn generate_timestamp_ns_array(rng: &mut StdRng) -> TimestampNanosecondArray {
TimestampNanosecondArray::from(
@@ -162,6 +163,31 @@ fn generate_duration_nanosecond_array(rng: &mut StdRng) -> DurationNanosecondArr
)
}
+// Durations under `i32::MAX` nanoseconds (about 2.1 seconds)
+fn generate_duration_nanosecond_small_array(rng: &mut StdRng) -> DurationNanosecondArray {
+ DurationNanosecondArray::from(
+ (0..BATCH_SIZE)
+ .map(|_| rng.random_range(0..i32::MAX as i64))
+ .collect::<Vec<_>>(),
+ )
+}
+
+fn generate_duration_nanosecond_array_with_nulls(
+ rng: &mut StdRng,
+) -> DurationNanosecondArray {
+ DurationNanosecondArray::from(
+ (0..BATCH_SIZE)
+ .map(|_| {
+ if rng.random_bool(NULL_DENSITY) {
+ None
+ } else {
+ Some(rng.random_range(0..TS_BOUND * 1_000_000_000))
+ }
+ })
+ .collect::<Vec<_>>(),
+ )
+}
+
fn bench_date_part(
c: &mut Criterion,
udf: &Arc<ScalarUDF>,
@@ -213,6 +239,10 @@ fn criterion_benchmark(c: &mut Criterion) {
let duration_ns = Arc::new(generate_duration_nanosecond_array(&mut rng)) as ArrayRef;
let date32 = Arc::new(generate_date32_array(&mut rng)) as ArrayRef;
let date64 = Arc::new(generate_date64_array(&mut rng)) as ArrayRef;
+ let duration_ns_small =
+ Arc::new(generate_duration_nanosecond_small_array(&mut rng)) as ArrayRef;
+ let duration_ns_nulls =
+ Arc::new(generate_duration_nanosecond_array_with_nulls(&mut rng)) as ArrayRef;
let udf = date_part();
@@ -322,6 +352,8 @@ fn criterion_benchmark(c: &mut Criterion) {
("interval_dt", &interval_dt),
("interval_mdn", &interval_mdn),
("duration_ns", &duration_ns),
+ ("duration_ns_small", &duration_ns_small),
+ ("duration_ns_nulls", &duration_ns_nulls),
] {
bench_date_part(
c,
diff --git a/datafusion/functions/src/datetime/date_part.rs b/datafusion/functions/src/datetime/date_part.rs
index 5b8d32062b..6dcb948a0e 100644
--- a/datafusion/functions/src/datetime/date_part.rs
+++ b/datafusion/functions/src/datetime/date_part.rs
@@ -39,7 +39,9 @@ use crate::datetime::common::parse_tz;
use datafusion_common::{
Result, ScalarValue,
cast::{
- as_date32_array, as_date64_array, as_int32_array, as_interval_dt_array,
+ as_date32_array, as_date64_array, as_duration_microsecond_array,
+ as_duration_millisecond_array, as_duration_nanosecond_array,
+ as_duration_second_array, as_int32_array, as_interval_dt_array,
as_interval_mdn_array, as_interval_ym_array, as_primitive_array,
as_time32_millisecond_array, as_time32_second_array, as_time64_microsecond_array,
as_time64_nanosecond_array, as_timestamp_microsecond_array,
@@ -504,48 +506,6 @@ fn seconds_as_i32(array: &dyn Array, unit: TimeUnit) -> Result<ArrayRef> {
}
}
-/// Invoke [`date_part`] on an `array` (e.g. Timestamp) and convert the
-/// result to a total number of seconds, milliseconds, microseconds or
-/// nanoseconds
-///
-/// Given epoch return f64, this is a duplicated function to optimize for f64 type
-fn seconds(array: &dyn Array, unit: TimeUnit) -> Result<ArrayRef> {
- let sf = match unit {
- Second => 1_f64,
- Millisecond => 1_000_f64,
- Microsecond => 1_000_000_f64,
- Nanosecond => 1_000_000_000_f64,
- };
- let secs = date_part(array, DatePart::Second)?;
- // This assumes array is primitive and not a dictionary
- let secs = as_int32_array(secs.as_ref())?;
- let subsecs = date_part(array, DatePart::Nanosecond)?;
- let subsecs = as_int32_array(subsecs.as_ref())?;
-
- // Special case where there are no nulls.
- if subsecs.null_count() == 0 {
- let r: Float64Array = binary(secs, subsecs, |secs, subsecs| {
- (secs as f64 + ((subsecs % 1_000_000_000) as f64 / 1_000_000_000_f64)) * sf
- })?;
- Ok(Arc::new(r))
- } else {
- // Nulls in secs are preserved, nulls in subsecs are treated as zero to account for the case
- // where the number of nanoseconds overflows.
- let r: Float64Array = secs
- .iter()
- .zip(subsecs)
- .map(|(secs, subsecs)| {
- secs.map(|secs| {
- let subsecs = subsecs.unwrap_or(0);
- (secs as f64 + ((subsecs % 1_000_000_000) as f64 / 1_000_000_000_f64))
- * sf
- })
- })
- .collect();
- Ok(Arc::new(r))
- }
-}
-
fn epoch(array: &dyn Array) -> Result<ArrayRef> {
const SECONDS_IN_A_DAY: f64 = 86400_f64;
// Note: Month-to-second conversion uses 30 days as an approximation.
@@ -589,7 +549,16 @@ fn epoch(array: &dyn Array) -> Result<ArrayRef> {
+ x.nanoseconds as f64 / 1_000_000_000_f64
})
}
- Duration(_) => return seconds(array, Second),
+ Duration(Second) => as_duration_second_array(array)?.unary(|x| x as f64),
+ Duration(Millisecond) => {
+ as_duration_millisecond_array(array)?.unary(|x| x as f64 / 1_000_f64)
+ }
+ Duration(Microsecond) => {
+ as_duration_microsecond_array(array)?.unary(|x| x as f64 / 1_000_000_f64)
+ }
+ Duration(Nanosecond) => {
+ as_duration_nanosecond_array(array)?.unary(|x| x as f64 / 1_000_000_000_f64)
+ }
d => return exec_err!("Cannot convert {d:?} to epoch"),
};
Ok(Arc::new(f))
diff --git a/datafusion/sqllogictest/test_files/datetime/date_part.slt b/datafusion/sqllogictest/test_files/datetime/date_part.slt
index 03269772d9..96cbe78d49 100644
--- a/datafusion/sqllogictest/test_files/datetime/date_part.slt
+++ b/datafusion/sqllogictest/test_files/datetime/date_part.slt
@@ -1178,6 +1178,78 @@ SELECT extract(epoch from arrow_cast(2, 'Duration(Second)'))
----
2
+# epoch keeps the fraction of a duration whose total sub-second units exceed i32::MAX
+query RR
+SELECT extract(epoch from arrow_cast(2147483647, 'Duration(Nanosecond)')),
+ extract(epoch from arrow_cast(2147483648, 'Duration(Nanosecond)'))
+----
+2.147483647 2.147483648
+
+# Reproducer from https://github.com/apache/datafusion/issues/26673: every row keeps its fraction.
+# Before the fix, the rows at or above 2^31 ns (2.147483648 s) lost their fraction.
+query TTR
+SELECT description,
+ arrow_typeof(finished_at - started_at),
+ extract(epoch from finished_at - started_at)
+FROM (VALUES
+ ('1.25 seconds', timestamp '2025-01-01 00:00:00', timestamp '2025-01-01 00:00:01.250000'),
+ ('2.10 seconds', timestamp '2025-01-01 00:00:00', timestamp '2025-01-01 00:00:02.100000'),
+ ('2.50 seconds', timestamp '2025-01-01 00:00:00', timestamp '2025-01-01 00:00:02.500000'),
+ ('4.763 seconds', timestamp '2025-01-01 00:00:00', timestamp '2025-01-01 00:00:04.763000'),
+ ('12.923350 seconds', timestamp '2025-01-01 00:00:00', timestamp '2025-01-01 00:00:12.923350')
+) AS t(description, started_at, finished_at)
+----
+1.25 seconds Duration(ns) 1.25
+2.10 seconds Duration(ns) 2.1
+2.50 seconds Duration(ns) 2.5
+4.763 seconds Duration(ns) 4.763
+12.923350 seconds Duration(ns) 12.92335
+
+query R
+SELECT extract(epoch from arrow_cast(2147483648, 'Duration(Microsecond)'))
+----
+2147.483648
+
+query R
+SELECT extract(epoch from arrow_cast(2147483648, 'Duration(Millisecond)'))
+----
+2147483.648
+
+query R
+SELECT extract(epoch from arrow_cast(2147483648, 'Duration(Second)'))
+----
+2147483648
+
+query R
+SELECT extract(epoch from arrow_cast(-2500000000, 'Duration(Nanosecond)'))
+----
+-2.5
+
+query R
+SELECT extract(epoch from arrow_cast(NULL, 'Duration(Nanosecond)'))
+----
+NULL
+
+# NULLs stay NULL and don't change the other rows of the batch
+query R
+SELECT extract(epoch from d) FROM (VALUES
+ (arrow_cast(2500000000, 'Duration(Nanosecond)')),
+ (NULL),
+ (arrow_cast(1500000000, 'Duration(Nanosecond)'))
+) AS t(d)
+----
+2.5
+NULL
+1.5
+
+query RRRR
+SELECT extract(epoch from arrow_cast(9223372036854775807, 'Duration(Nanosecond)')),
+ extract(epoch from arrow_cast(-9223372036854775807, 'Duration(Nanosecond)')),
+ extract(epoch from arrow_cast(9223372036854775807, 'Duration(Second)')),
+ extract(epoch from arrow_cast(-9223372036854775807, 'Duration(Second)'))
+----
+9223372036.854776 -9223372036.854776 9223372036854776000 -9223372036854776000
+
query I
SELECT extract(millisecond from arrow_cast(2, 'Duration(Second)'))
----
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org