andygrove opened a new issue, #23891:
URL: https://github.com/apache/datafusion/issues/23891
### Describe the bug
`datafusion-spark`'s `next_day` computes its result with `chrono::NaiveDate`
arithmetic in `datafusion/spark/src/function/datetime/next_day.rs`:
```rust
fn next_date_for_day_of_week(days: i32, weekday: Weekday) -> Option<i32> {
let date = Date32Type::to_naive_date_opt(days)?;
let delta = (7 - date.weekday().days_since(weekday)) as i64;
Some(Date32Type::from_naive_date(
date.checked_add_signed(Duration::days(delta))?,
))
}
```
`chrono::NaiveDate::MAX` is `+262142-12-31`, epoch day `95026236`. A result
that would land past it is not representable, so the function returns NULL.
Spark has no such limit. `DateTimeUtils.getNextDateForDayOfWeek` is pure
`Int`
arithmetic on the epoch day:
```scala
def getNextDateForDayOfWeek(startDay: Int, dayOfWeek: Int): Int = {
startDay + 1 + ((dayOfWeek - 1 - startDay) % 7 + 7) % 7
}
```
So Spark keeps producing a value for every start day up to `Int.MaxValue`,
including the range where DataFusion now returns NULL.
Affected start days are `95026230` through `95026236`, and which of them
diverge depends on the requested weekday. A start date already on the
requested weekday advances a full week, so it overflows from six days earlier
than one that advances a single day.
### To Reproduce
DataFusion SQL:
```sql
SELECT next_day(arrow_cast(95026236, 'Date32'), 'Mon'::string);
-- NULL
SELECT next_day(arrow_cast(95026230, 'Date32'), 'Tue'::string);
-- NULL
```
Spark SQL. Note that these have to be read back as a string, because
`collect()` in PySpark cannot convert a date this far out to a
`datetime.date`:
```sql
SELECT CAST(next_day(CAST(95026236 AS DATE), 'Mon') AS STRING);
SELECT CAST(next_day(CAST(95026230 AS DATE), 'Tue') AS STRING);
```
Spark returns a date seven days after the start day in both cases rather than
NULL.
### Expected behavior
Ideally `next_day` matches Spark and returns a value for every `Int` start
day. That requires computing on the epoch day directly rather than routing
through `NaiveDate`, which is straightforward for the arithmetic itself but
raises the question of what the rest of DataFusion does with a `Date32` value
it cannot render.
### Additional context
This is the residual half of a panic fix. Before the fix, the same inputs
panicked with `` `NaiveDate + TimeDelta` overflowed ``, reachable from plain
SQL and from any `Date32` column holding such a value:
```sql
SELECT next_day(arrow_cast(95026236, 'Date32'), 'Mon'::string);
-- thread panicked: `NaiveDate + TimeDelta` overflowed
```
The panic was fixed by switching to `checked_add_signed`, which trades the
panic for a NULL. NULL is strictly better than a panic, so the minimal fix
was taken. Closing the remaining gap between NULL and Spark's value is a
separate change, because it is a representation question rather than a bug in
the day-of-week logic, and it may need a decision about how far DataFusion
wants to follow Spark outside the range `chrono` can express.
The boundary cases are asserted in
`datafusion/sqllogictest/test_files/spark/datetime/next_day.slt` under the
"Boundary dates" heading, with a comment naming the divergence. They assert
DataFusion's current NULL, so they will need updating if this is fixed.
Relevant file and line:
`datafusion/spark/src/function/datetime/next_day.rs`,
`next_date_for_day_of_week`.
Surfaced by the audit-datafusion-spark-expression skill.
--
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]
