Skip to content

fix: support CAST/TRY CAST expressions for return_field_from_args - #24334

Open
dd-annarose wants to merge 1 commit into
apache:mainfrom
dd-annarose:annarose/constant-date-part
Open

fix: support CAST/TRY CAST expressions for return_field_from_args#24334
dd-annarose wants to merge 1 commit into
apache:mainfrom
dd-annarose:annarose/constant-date-part

Conversation

@dd-annarose

@dd-annarose dd-annarose commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Functions like date_part require the value of one of their arguments to determine their return type.

This PR adds support for CAST/TRY_CAST in addition to Literal values.

This change is required as constant-folding cannot run before analysis. It casts logical fields once (not per row) so there is no impact in terms of performance.

Are these changes tested?

Yes, date_part.slt has been updated with two tests.

Are there any user-facing changes?

No.

@github-actions github-actions Bot added logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt) labels Aug 13, 2026
@dd-annarose
fix: support CAST/TRY CAST expressions for return_field_from_args
14b61b5 
Functions like date_part require the value of one of their arguments to determine their return type.

This commit adds support for CAST/TRY_CAST in addition to Literal values.

This change is required as constant-folding cannot run before analysis.
It casts logical fields once (not per row) so there is no impact in terms of performance.
@dd-annarose
dd-annarose force-pushed the annarose/constant-date-part branch from bcc580c to 14b61b5 Compare August 13, 2026 16:20
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.14%. Comparing base (9b3b518) to head (14b61b5).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #24334      +/-   ##
==========================================
- Coverage   81.29%   81.14%   -0.16%     
==========================================
  Files        1110     1112       +2     
  Lines      385336   386940    +1604     
  Branches   385336   386940    +1604     
==========================================
+ Hits       313261   313967     +706     
- Misses      53594    54478     +884     
- Partials    18481    18495      +14

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

@Jefffrey

Jefffrey commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

i dont think the linked issue is correct here?

i also dont see the rationale for this change, it seems odd to allow a cast expression when we expect a literal; is there a usecase that requires this?

@dd-annarose

dd-annarose commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@Jefffrey ah yes, the issue is not correct, I misread.

Postgres does support SELECT date_part(CAST('year' as VARCHAR), now()); and it feels like a good addition to DataFusion. If constant-folding was happening before date_part had to determine its return type, it wouldn't be an issue; so it seems like a good addition.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dd-annarose @codecov-commenter @Jefffrey