[fix](fe) Fix datediff folding for zero date#64084
Open
feiniaofeiafei wants to merge 2 commits into
Open
Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: FE constant folding for DATEDIFF used Java proleptic date arithmetic, while BE runtime evaluates DATEDIFF by subtracting Doris day numbers. For zero-date inputs such as 0000-01-01, Java date arithmetic disagrees with Doris day-number semantics, so folded DATEDIFF could return a different result from runtime execution and from TO_DAYS(a) - TO_DAYS(b). This change reuses the FE day-number calculation used by TO_DAYS for DATEDIFF folding, keeping folded and runtime semantics consistent for zero-date boundary cases.
### Release note
None
### Check List (For Author)
- Test: Unit Test
- Attempted: ./run-fe-ut.sh --run org.apache.doris.nereids.rules.expression.FoldConstantTest
- Result: failed before running tests because current FE compile fails in LogicalPlanBuilder.java with missing ctx.HOT(); the DATEDIFF-specific compile errors found in the first run were fixed and did not recur.
- Behavior changed: No
- Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29093 ms |
Contributor
TPC-DS: Total hot run time: 168752 ms |
Contributor
TPC-H: Total hot run time: 29560 ms |
Contributor
TPC-DS: Total hot run time: 172066 ms |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
Author
|
run p0 |
Contributor
Author
|
run cloud_p0 |
Contributor
Author
|
run external |
Contributor
Author
|
run nonConcurrent |
Contributor
FE Regression Coverage ReportIncrement line coverage |
3 similar comments
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
|
/review |
morrySnow
approved these changes
Jun 5, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code review completed. I did not find any blocking correctness issue in this PR.
Critical checkpoint conclusions:
- Goal and test: The PR aims to make Nereids constant folding for
datediffuse Doris day-number semantics instead of Java date arithmetic. The change accomplishes this for the modified DateV2/DateTimeV2 folding paths, and the added FE unit cases cover the reported0000-01-01boundary for DateV2 and DateTimeV2 operands. - Scope and clarity: The change is small and focused. It reuses the existing FE
to_daysday-number calculation rather than adding a separate divergent implementation. - Concurrency and lifecycle: No concurrency, locking, static initialization, or non-trivial lifecycle behavior is introduced.
- Configuration and compatibility: No new configuration, storage format, FE-BE protocol, or incompatible behavior is introduced.
- Parallel paths: BE runtime
datediffsubtractsdaynr()values, and the exposed FE helper mirrors BEcalc_daynrspecial handling. I did not find another changed path that needs the same modification in this PR. - Conditional logic: The zero-date special handling already exists in the shared day-number helper and matches BE semantics.
- Test coverage: Coverage is limited to FE unit tests and specifically includes the zero-date folding case. I did not run tests locally in this review.
- Observability: No new observability is needed for this constant-folding change.
- Transaction, persistence, and data writes: Not applicable.
- Performance and memory: The new logic is constant-time and does not introduce material allocation or memory-tracking concerns.
User focus response: No additional user-provided review focus was supplied.
Contributor
FE Regression Coverage ReportIncrement line coverage |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: FE constant folding for DATEDIFF used Java proleptic date arithmetic, while BE runtime evaluates DATEDIFF by subtracting Doris day numbers. For zero-date inputs such as 0000-01-01, Java date arithmetic disagrees with Doris day-number semantics, so folded DATEDIFF could return a different result from runtime execution and from TO_DAYS(a) - TO_DAYS(b). This change reuses the FE day-number calculation used by TO_DAYS for DATEDIFF folding, keeping folded and runtime semantics consistent for zero-date boundary cases.
Release note
None
Check List (For Author)
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)