fix(spark): next_day should not trim whitespace from dayOfWeek#22734
fix(spark): next_day should not trim whitespace from dayOfWeek#22734koopatroopa787 wants to merge 1 commit into
Conversation
Spark's next_day function performs exact (case-insensitive) matching on the dayOfWeek argument — whitespace-padded values like ' MO ' are not valid day names and should return NULL, not be silently accepted. Remove the .trim() call from spark_next_day so that ' MO ', ' Monday ', and ' sun ' all return NULL, matching Spark's actual behavior. Adds three sqllogictest cases that previously returned a date but now correctly return NULL. Closes apache#22717
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Aligns next_day Spark-compat behavior by treating padded (whitespace-surrounded) day name inputs as invalid, matching Spark’s strict parsing.
Changes:
- Add Sqllogictest cases asserting padded day names return
NULL - Update
spark_next_dayimplementation to stop trimmingday_of_weekbefore matching
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| datafusion/sqllogictest/test_files/spark/datetime/next_day.slt | Adds regression tests for padded day-name inputs returning NULL to match Spark behavior |
| datafusion/spark/src/function/datetime/next_day.rs | Removes .trim() so whitespace is no longer ignored when parsing day names |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let date = Date32Type::to_naive_date_opt(days)?; | ||
|
|
||
| let day_of_week = day_of_week.trim().to_uppercase(); | ||
| let day_of_week = day_of_week.to_uppercase(); |
|
@koopatroopa787 another PR has already been approved for this issue #22720 I would appreciate if you first check if there are open PRs for a particular issue before moving forward. |
|
Hey @kumarUjjawal, really sorry about this — I missed #22720 when I checked the issue. Closing this one out. Thanks for the heads up, I'll make sure to check for existing PRs before opening one next time. |
Thank you! Appreciate it. |
Which issue does this PR close?
Closes #22717
Rationale for this change
Spark's
next_dayfunction performs exact (case-insensitive) matching on thedayOfWeekargument. Whitespace-padded values like' MO 'are not valid day names in Spark and should returnNULL. DataFusion was calling.trim()before matching, so' MO 'was silently accepted and returned a date — diverging from Spark's behavior.What changes are included in this PR?
datafusion/spark/src/function/datetime/next_day.rs: removed.trim()from thespark_next_dayhelper so whitespace-padded day names fall through to theNonebranch.datafusion/sqllogictest/test_files/spark/datetime/next_day.slt: added three regression tests (' MO ',' Monday ',' sun ') that all now correctly returnNULL.Are there any user-facing changes?
Yes —
next_day(date, ' MO ')now returnsNULLinstead of a date. This is a bug fix that aligns with Spark's actual behavior.🤖 Generated with Claude Code