Skip to content

Unify temporal field parsing with DatePart - #24906

Open
sdf-jkl wants to merge 1 commit into
apache:mainfrom
sdf-jkl:issue-22033-unify-temporal-field-parsing
Open

Unify temporal field parsing with DatePart#24906
sdf-jkl wants to merge 1 commit into
apache:mainfrom
sdf-jkl:issue-22033-unify-temporal-field-parsing

Conversation

@sdf-jkl

@sdf-jkl sdf-jkl commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Temporal functions currently parse field names independently, which produces inconsistent user-visible behavior. For example, date_part accepts aliases such as mon, mons, month, and months, while date_trunc accepts only month.

Using Arrow's DatePart parser as the common representation makes these aliases consistent across temporal functions and SQL unparsing.

What changes are included in this PR?

  • Use DatePart::from_str directly in date_part instead of parsing through IntervalUnit and maintaining additional manual mappings.
  • Replace the private DateTruncGranularity enum and its string parser with DatePart.
  • Restrict date_trunc to the DatePart variants that represent valid truncation granularities.
  • Update the Spark wrappers to delegate field parsing to the shared implementation while retaining their function-specific semantic restrictions.
  • Map Spark's Sunday-based dayofweek directly to dow1 instead of adding one to the result expression.
  • Use DatePart when canonicalizing temporal fields in the SQL unparser.

What is the testing strategy for this PR?

SQL logic tests cover the newly shared aliases, including:

  • date_part: QUARTERS, ISOWEEK, DAYOFYEAR, and DAYOFWEEK
  • date_trunc: mon, months, qtr, and yrs
  • Spark wrappers: QUARTERS, DAYOFYEAR, MONTHS, and MINS
  • Unparser canonicalization of aliases such as MONS and YRS

The following checks passed:

  • cargo fmt --all
  • cargo clippy --all-targets --all-features -- -D warnings
  • Extended workspace tests with avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption
  • Spark datetime SQL logic tests
  • cargo test -p datafusion-spark --lib

Are there any user-facing changes?

Yes. Temporal functions now consistently accept the field-name aliases recognized by Arrow's DatePart parser. This broadens accepted input spellings without changing existing accepted inputs or truncation behavior.

The Spark wrappers inherit these aliases as well, making them slightly more permissive than Spark's exact spelling tables.

@github-actions github-actions Bot added sql SQL Planner sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation spark labels Sep 3, 2026
@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.16541% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.63%. Comparing base (1608ee7) to head (ea29199).
⚠️ Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/functions/src/datetime/date_trunc.rs 62.22% 34 Missing ⚠️
datafusion/sql/src/unparser/utils.rs 57.14% 6 Missing ⚠️
datafusion/functions/src/datetime/date_part.rs 64.28% 0 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24906      +/-   ##
==========================================
+ Coverage   81.61%   81.63%   +0.01%     
==========================================
  Files        1123     1123              
  Lines      409562   409685     +123     
  Branches   409562   409685     +123     
==========================================
+ Hits       334284   334447     +163     
+ Misses      55647    55587      -60     
- Partials    19631    19651      +20     

☔ 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.

@kumarUjjawal kumarUjjawal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation spark sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify granularity / field-name parsing across temporal functions

3 participants