Skip to content

perf: parse next_day's day-of-week without allocating, and once per batch - #23885

Closed
andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:opt/spark-next-day
Closed

andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:opt/spark-next-day

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A

Rationale for this change

spark_next_day did a surprising amount of work per row to turn a day name into
a Weekday:

let day_of_week = day_of_week.to_uppercase();          // heap allocation + Unicode casing
let day_of_week = match day_of_week.as_str() {
    "MO" | "MON" | "MONDAY" => Some("MONDAY"),          // 21 literals
    ...
};
let day_of_week = day_of_week.parse::<Weekday>();       // second case-insensitive match

So every row allocated a String, matched it against 21 literals to produce
another string literal, and then re-parsed that literal back into the enum the
match had already identified.

On top of that, next_day(col, 'MONDAY') — the common form, where the day name
is a constant — ran all of the above once per row even though the answer is the
same for the whole batch.

What changes are included in this PR?

  • The literal match now yields a Weekday directly, so the round-trip through
    "MONDAY" and parse::<Weekday>() is gone.
  • parse_day_of_week upper-cases ASCII input in a [u8; 9] stack buffer
    ("WEDNESDAY" is the longest accepted name, so anything longer cannot match
    and returns early). No allocation on this path.
  • Non-ASCII input still goes through str::to_uppercase. That is deliberate:
    to_uppercase is Unicode-aware, matching Spark's toUpperCase(Locale.ROOT),
    and handling it with ASCII casing would change which strings match. For
    example U+017F (ſ, long s) upper-cases to S, so "ſun" is accepted by
    Spark and by this function, on both the old and new code.
  • spark_next_day now takes a Weekday rather than a &str, which lets the
    array-plus-scalar branch parse the name once before the loop instead of inside
    unary_opt.

No change to which inputs are accepted or what they return.

Are these changes tested?

spark/datetime/next_day.slt asserts concrete dates across abbreviations, full
names, mixed case, invalid names, and NULL inputs; all 53 spark/datetime
sqllogictest files pass, along with the 261 datafusion-spark unit tests.

The existing next_day_rejects_whitespace_padded_day_names test now targets
parse_day_of_week directly, since that is where the rejection lives. Three
tests were added:

  • parse_day_of_week_is_case_insensitive — abbreviations and full names in
    lower, upper, and mixed case.
  • parse_day_of_week_rejects_unknown_names — empty, too short, too long
    (past the stack-buffer bound), non-day words, and non-ASCII.
  • parse_day_of_week_matches_unicode_uppercasing — pins that the ASCII fast
    path and the Unicode fallback accept the same set, using the ſ → S case
    above. This is the test that would catch the fast path silently narrowing
    what next_day accepts.

The benchmark used below is added separately in #23882, so the baseline can be
measured on main before this change lands.

Benchmarks

Criterion, apache/main @ f1ab86dad as baseline. Median of the reported
change interval.

Benchmark 1024 8192
next_day/scalar_day −47.2% −46.0%
next_day/array_day −37.6% −33.7%

scalar_day is next_day(col, 'MONDAY'); array_day passes the day name as a
column, so it still parses per row and gains only from the allocation removal.

Are there any user-facing changes?

No. next_day accepts the same inputs and returns the same results.

…atch

spark_next_day called day_of_week.to_uppercase() on every row -- a heap
allocation plus full Unicode case mapping -- matched the result against 21
string literals, mapped that to another literal, then parsed that literal back
into a Weekday with a second case-insensitive match.

The match now yields a Weekday directly, and ASCII input is upper-cased in a
stack buffer. Non-ASCII input keeps the allocating to_uppercase path, since
that is Unicode-aware and dropping it would change which strings match.

For next_day(col, 'MONDAY') the name is constant across the batch, so it is
now parsed once before the loop rather than on every row.

Scalar day-of-week -47%, column day-of-week -34%, against the benchmark added
in apache#23882.
@codecov-commenter

codecov-commenter commented Jul 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.83051% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.65%. Comparing base (f1ab86d) to head (5e810e7).
⚠️ Report is 217 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/spark/src/function/datetime/next_day.rs 89.83% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #23885    +/-   ##
========================================
  Coverage   80.65%   80.65%            
========================================
  Files        1091     1092     +1     
  Lines      371031   371135   +104     
  Branches   371031   371135   +104     
========================================
+ Hits       299256   299344    +88     
- Misses      53935    53943     +8     
- Partials    17840    17848     +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@andygrove
andygrove marked this pull request as draft July 25, 2026 17:19
@andygrove

Copy link
Copy Markdown
Member Author

Moving to draft. Fixing some bugs first.

@andygrove andygrove closed this Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

spark Changes to datafusion-spark crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants