You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/07/25 14:39:46 UTC
[PR] perf: parse next_day's day-of-week without allocating, and once per batch [datafusion]
andygrove opened a new pull request, #23885:
URL: https://github.com/apache/datafusion/pull/23885
## 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`:
```rust
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: parse next_day's day-of-week without allocating, and once per batch [datafusion]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #23885:
URL: https://github.com/apache/datafusion/pull/23885#issuecomment-5079452646
Moving to draft. Fixing some bugs first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: parse next_day's day-of-week without allocating, and once per batch [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #23885:
URL: https://github.com/apache/datafusion/pull/23885#issuecomment-5079006709
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/23885?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `89.83051%` with `6 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 80.65%. Comparing base ([`f1ab86d`](https://app.codecov.io/gh/apache/datafusion/commit/f1ab86dad406a189e43ac19d24965b3fbf9dbba9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5e810e7`](https://app.codecov.io/gh/apache/datafusion/commit/5e810e7c09b8c82dd729634995e2e6d6ee177f36?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/23885?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/spark/src/function/datetime/next\_day.rs](https://app.codecov.io/gh/apache/datafusion/pull/23885?src=pr&el=tree&filepath=datafusion%2Fspark%2Fsrc%2Ffunction%2Fdatetime%2Fnext_day.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9zcGFyay9zcmMvZnVuY3Rpb24vZGF0ZXRpbWUvbmV4dF9kYXkucnM=) | 89.83% | [5 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/23885?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ 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
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/23885?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: parse next_day's day-of-week without allocating, and once per batch [datafusion]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove closed pull request #23885: perf: parse next_day's day-of-week without allocating, and once per batch
URL: https://github.com/apache/datafusion/pull/23885
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org