viirya opened a new pull request, #24194:
URL: https://github.com/apache/datafusion/pull/24194
## Which issue does this PR close?
- Closes #23891.
## Rationale for this change
`datafusion-spark`'s `next_day` computed its result by adding the day offset
to a `chrono::NaiveDate`:
```rust
Some(Date32Type::from_naive_date(
date + Duration::days((7 - date.weekday().days_since(day_of_week)) as
i64),
))
```
When the next occurrence of the requested weekday lands past
`chrono::NaiveDate::MAX` (epoch day `95026236`) — which happens for start dates
in the last week of the representable `Date32` range — the `NaiveDate +
TimeDelta` add **panics** with `NaiveDate + TimeDelta overflowed`. (The linked
issue reports NULL; the scalar path in fact panics.)
Spark's `DateTimeUtils.getNextDateForDayOfWeek` is pure integer arithmetic
on the epoch day and always produces a value:
```scala
def getNextDateForDayOfWeek(startDay: Int, dayOfWeek: Int): Int = {
startDay + 1 + ((dayOfWeek - 1 - startDay) % 7 + 7) % 7
}
```
## What changes are included in this PR?
- Compute the result on the epoch day directly (`days + delta`, via
`checked_add`) instead of building a `NaiveDate` for the result, so a next
occurrence past `NaiveDate::MAX` returns a value instead of panicking. The
weekday/offset logic is unchanged, so results for all in-range dates are
identical.
Scope note: a *start* day beyond `NaiveDate::MAX` (`> 95026236`) still
returns NULL, because the weekday is derived via `NaiveDate`. Extending full
`Int`-range parity (computing the weekday arithmetically too) is a larger
change left as a follow-up; this PR fixes the panic, which is the concrete
reported harm.
## Are these changes tested?
Yes.
- Unit test `next_day_handles_far_future_start_dates` covering the two cases
from the issue (`95026236`/`Mon`, `95026230`/`Tue`), each of which panicked
before this change.
- SLT coverage in `spark/datetime/next_day.slt` asserting the same two cases
(casting the `Date32` result to `Int32`, since these dates are past the
printable range).
## Are there any user-facing changes?
`next_day` no longer panics for far-future start dates; it returns the
correct epoch day. No API changes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
