speedup `date_trunc` (~7x faster) in some cases #16859

waynexia · 2025-07-22T23:02:19Z

Which issue does this PR close?

follow-up of Speedup date_trunc (~20% time reduction) #14593

Rationale for this change

Follows the comment #14593 (comment) to implement an array version of date_trunc.

While implementing, I found that there is a lot of code handling boring calendar things, which is totally unrelated to some small granularities (less or equal to "hour"). So I removed them as well. Benchmark shows this simplification can bring 4x performance.

I also updated the function document BTW. Include millisecond and microsecond to supported granularity list.

What changes are included in this PR?

a faster implementation for date_trunc with granularities from "microsecond" to "hour"

date_trunc_minute_1000  time:   [16.600 µs 16.643 µs 16.684 µs]
Found 8 outliers among 100 measurements (8.00%)
  8 (8.00%) low mild

date_trunc_minute_1000  time:   [2.3474 µs 2.3519 µs 2.3579 µs]
                        change: [-85.946% -85.909% -85.870%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe

Are these changes tested?

using existing unit tests

Are there any user-facing changes?

Signed-off-by: Ruihang Xia <[email protected]>

findepi · 2025-07-23T12:14:36Z

datafusion/functions/src/datetime/date_trunc.rs

+                granularity.as_str(),
+                "second" | "minute" | "millisecond" | "microsecond"
+            ) || (parsed_tz.is_none() && granularity.as_str() == "hour")
+            {
+                let result = general_date_trunc_array_fine_granularity(


Does this assume zone offsets are multiples of a whole hour?

(for example, Asia/Kathmandu is +05:45)

The parsed_tz.is_none() check ensures this array is not associated with any timezone (hence the default UTC) and is safe to do in this way.

Added a test case f50fb3f for this case (TIL

The parsed_tz.is_none() check

~~it's there for hour, but not for minute.~~
~~Why have different zone-related conditions for hour and minute?~~

Because (AFAIK) no timezone has a different, non-integer offset on minute like +04:32:10, so we can truncate all timestamps at minute level regardless of their timezone

You're right. For minute and smaller granularities the new code path is always fine¹
For hour it's also fine, for zone-less data.

(¹) in the past (early XX century and earlier) named zones often had offset changes not multiple of minutes at list this is what https://www.timeanddate.com/time/zone/nepal/kathmandu shows me. Not sure whether chrono respects that

for zone-less data (timestamp without time zone), day granularity can also be applied with just divide and mul, just like hour. Or am i blind again?

SELECT date_trunc('minute', arrow_cast('1910-01-01T00:00:00Z', 'Timestamp(Second, Some("Asia/Kathmandu"))')); SELECT date_trunc('minute', arrow_cast(v, 'Timestamp(Second, Some("Asia/Kathmandu"))')) FROM (VALUES ('1910-01-01T00:00:00Z')) t(v);

+-----------------------------------------------------------------------------------------------------------------------+ | date_trunc(Utf8("minute"),arrow_cast(Utf8("1910-01-01T00:00:00Z"),Utf8("Timestamp(Second, Some("Asia/Kathmandu"))"))) | +-----------------------------------------------------------------------------------------------------------------------+ | 1910-01-01T05:41:16+05:41 | +-----------------------------------------------------------------------------------------------------------------------+ 1 row(s) fetched. Elapsed 0.065 seconds. +----------------------------------------------------------------------------------------------+ | date_trunc(Utf8("minute"),arrow_cast(t.v,Utf8("Timestamp(Second, Some("Asia/Kathmandu"))"))) | +----------------------------------------------------------------------------------------------+ | 1910-01-01T05:41:16+05:41 | +----------------------------------------------------------------------------------------------+ 1 row(s) fetched. Elapsed 0.012 seconds.

this doesn't look correct to me
however, this is the same result as on main, so it's not a regression.
Is it because the existing date_trunc code do similar logic as the new optimized code path?

It doesn't look right to me either. First, it's not truncating to the minute, and second I have my doubts that TZ is correct with that time. I'd have to think about that a bit more but I think a ticket should be filed to have that looked at some more

Not sure whether chrono respects that

it looks it does

let timestamp = chrono::DateTime::parse_from_rfc3339("1910-01-01T00:00:00Z").unwrap(); dbg!(&timestamp); let tz = chrono_tz::Asia::Kathmandu; // let tz = arrow::array::timezone::Tz::from_str("Asia/Kathmandu").unwrap(); dbg!(&tz); let zoned_date_time = timestamp.with_timezone(&tz); dbg!(&zoned_date_time); let local_date_time = zoned_date_time.naive_local(); dbg!(&local_date_time); let truncated_local = local_date_time.with_second(0).unwrap(); dbg!(&truncated_local); let mapped_back = match truncated_local.and_local_timezone(tz) { MappedLocalTime::Single(mapped) => mapped, _ => panic!(), }; dbg!(&mapped_back); let timestamp = mapped_back.to_utc(); dbg!(&timestamp); dbg!(timestamp.timestamp());

[tests/chrono.rs:7:1] &timestamp = 1910-01-01T00:00:00+00:00 [tests/chrono.rs:10:1] &tz = Asia/Kathmandu [tests/chrono.rs:12:1] &zoned_date_time = 1910-01-01T05:41:16LMT [tests/chrono.rs:14:1] &local_date_time = 1910-01-01T05:41:16 [tests/chrono.rs:16:1] &truncated_local = 1910-01-01T05:41:00 [tests/chrono.rs:21:1] &mapped_back = 1910-01-01T05:41:00LMT [tests/chrono.rs:23:1] &timestamp = 1909-12-31T23:59:44Z [tests/chrono.rs:24:1] timestamp.timestamp() = -1893456016

however either us, or arrow, makes assumption that offsets are whole minutes. this is visible in the output above (https://github.com/apache/datafusion/pull/16859/files#r2229281161). maybe that's rendering in the CLI output rendering layer. it makes reasoning about this harder.

SELECT cast(date_trunc('minute', arrow_cast(v, 'Timestamp(Second, Some("Asia/Kathmandu"))')) as bigint) FROM (VALUES ('1910-01-01T00:00:00Z')) t(v);

main: -1893456000_000000000

PR: -1893456000_000000000

while per-chrono above, the correct result seems to be -1893456016_000000000

Thank you for your very detailed check and explanation. I find another example with an extra search (that website is great)...

Signed-off-by: Ruihang Xia <[email protected]>

alamb · 2025-07-24T17:27:52Z

🤖 ./gh_compare_branch_bench.sh Benchmark Script Running
Linux aal-dev 6.11.0-1016-gcp #16~24.04.1-Ubuntu SMP Wed May 28 02:40:52 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing date-trunc-fast (f50fb3f) to a0ce581 diff
BENCH_NAME=date_trunc
BENCH_COMMAND=cargo bench --bench date_trunc
BENCH_FILTER=
BENCH_BRANCH_NAME=date-trunc-fast
Results will be posted here when complete

alamb

Thank you @waynexia -- this makes sense to me

Thank you @findepi for the good review question

alamb · 2025-07-24T17:32:05Z

🤖: Benchmark completed

Details

group                     date-trunc-fast                        main
-----                     ---------------                        ----
date_trunc_minute_1000    1.00      4.7±0.01µs        ? ?/sec    6.17     28.7±0.03µs        ? ?/sec

findepi

Let's add a code comment

findepi · 2025-07-24T20:51:38Z

datafusion/functions/src/datetime/date_trunc.rs

+            // fast path for fine granularities
+            if matches!(
+                granularity.as_str(),
+                "second" | "minute" | "millisecond" | "microsecond"


minutes is correct for modern zones, but not historical dates.
The old code apparently has some issues with them too though (https://github.com/apache/datafusion/pull/16859/files#r2229547803)
Let's add a code comment about our conscious ignorance of historical zone offsets here.

Add some comments in 346ae59

findepi · 2025-07-24T20:55:33Z

datafusion/functions/src/datetime/date_trunc.rs

+            if matches!(
+                granularity.as_str(),
+                "second" | "minute" | "millisecond" | "microsecond"
+            ) || (parsed_tz.is_none() && granularity.as_str() == "hour")


let's unlock the optimization for "day" in zoneless case.

make sense 👍 346ae59

Signed-off-by: Ruihang Xia <[email protected]>

waynexia · 2025-07-28T21:43:53Z

Appreciate your detailed review! Learned a lot about timezone 🫨

findepi · 2025-08-06T09:42:10Z

datafusion/functions/src/datetime/date_trunc.rs

+            // fast path for fine granularities
+            if matches!(
+                granularity.as_str(),
+                // For morden timezones, it's correct to truncate "minute" in this way.


morden -> modern?

fix them all 😎 #17135

speedup (~7x faster in some cases)

288b7b7

Signed-off-by: Ruihang Xia <[email protected]>

github-actions bot added the functions Changes to functions implementation label Jul 22, 2025

update function document

6b9de5b

Signed-off-by: Ruihang Xia <[email protected]>

github-actions bot added the documentation Improvements or additions to documentation label Jul 22, 2025

findepi reviewed Jul 23, 2025

View reviewed changes

add a case for Asia/Kathmandu

f50fb3f

Signed-off-by: Ruihang Xia <[email protected]>

alamb added the performance Make DataFusion faster label Jul 24, 2025

alamb approved these changes Jul 24, 2025

View reviewed changes

findepi approved these changes Jul 24, 2025

View reviewed changes

findepi reviewed Jul 24, 2025

View reviewed changes

add day to fast path and some comments

346ae59

Signed-off-by: Ruihang Xia <[email protected]>

waynexia merged commit 764d547 into apache:main Jul 28, 2025
29 checks passed

waynexia deleted the date-trunc-fast branch July 28, 2025 21:42

Standing-Man pushed a commit to Standing-Man/datafusion that referenced this pull request Aug 4, 2025

speedup date_trunc (~7x faster) in some cases (apache#16859)

95f3d0d

findepi reviewed Aug 6, 2025

View reviewed changes

waynexia mentioned this pull request Aug 12, 2025

chore: fix typos #17135

Open

speedup date_trunc (~7x faster) in some cases #16859

speedup date_trunc (~7x faster) in some cases #16859

Uh oh!

Conversation

waynexia commented Jul 22, 2025 • 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 there any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

findepi Jul 24, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb commented Jul 24, 2025

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb commented Jul 24, 2025

Uh oh!

findepi 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

waynexia commented Jul 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

speedup `date_trunc` (~7x faster) in some cases #16859

speedup `date_trunc` (~7x faster) in some cases #16859

waynexia commented Jul 22, 2025 •

edited

Loading

findepi Jul 24, 2025 •

edited

Loading