jacktengg opened a new pull request, #70485:
URL: https://github.com/apache/doris/pull/70485
### Background
TIMESTAMPTZ stores a UTC date and time. Calendar-year arithmetic must use
the date in the session time zone. DATE and DATETIME already use February 28
when a leap-day target is not in a leap year.
### What problem does this PR solve?
Issue Number: None
Related PR: None
PR Type: Bugfix
Problem Summary:
For 2024-02-29 00:00:00 in +08:00, years_add(ts, 1) and years_sub(ts, 1)
returned March 1. The stored UTC date is February 28, so UTC year arithmetic
does not apply the local leap-day rule. Compute the year change on the local
date in both FE constant evaluation and BE column evaluation, then convert the
result to UTC.
### Details
- **Change map:** 313 changed lines: 86 production, 211 tests, 16 generated
output, and 0 documentation. BE uses the session calendar and passes the
context through the column paths (57 lines). FE applies the same calendar and
DST rules during constant evaluation (29 lines).
- **Frequency:** Deterministic when a year change in UTC differs from the
required change on the local calendar date.
- **Root cause:** Both paths changed the stored UTC year. For the reported
input, the resulting February 28 at 16:00 UTC becomes March 1 at 00:00 in
+08:00.
- **Why existing tests missed it:** Year tests covered DATE/DATETIME and a
UTC leap day. They did not cover a local leap day whose UTC date is February 28.
- **Fix:** Reuse the DATETIME year transform in BE after conversion to the
session zone. Apply the same conversion in FE. Keep microseconds, preserve a
valid input offset in a repeated hour, and reject dates outside the supported
range.
- **Other paths and branches:** Cover addition, subtraction, negative
intervals, zero intervals, and the DATE_ADD/DATE_SUB YEAR forms. No Thrift,
Protobuf, storage-format, or configuration change is required.
- **Behavior change:** TIMESTAMPTZ year arithmetic uses the session calendar
date and clamps a leap day to February 28 when the target year is not a leap
year.
### Testing
Local validation:
- ASAN BE build and FE build passed. FE Checkstyle passed.
- run-be-ut.sh: 5 relevant year-arithmetic tests passed.
- run-fe-ut.sh: all 16 TimestampTzLiteralTest tests passed. With the
original UTC year calculation restored, the new leap-day and DST-gap tests
failed; both passed after the fix.
- run-regression-test.sh: the one-row test_timestamptz_year_interval suite
passed. The script generated the .out file; its dates, times, fractions, and
offsets were checked. A second run compared against this file and passed (1
suite, 0 failures).
- clang-format 16, clang-tidy, and build hygiene passed. Source whitespace
checks passed. The generated .out file keeps the regression runner's final
blank separator.
### Release note
Fix TIMESTAMPTZ year arithmetic for local leap days. Adding or subtracting a
year now returns February 28 when the target year has no February 29. For these
leap-day inputs, the time and fractional seconds are preserved.
### Check List (For Author)
- Behavior changed: Yes (see Details)
- Does this need documentation: No
- Pre-submission status: Ready
### AI Disclosure
- Tool: Codex, GPT-6
- Generated: Code, tests, and this description
- Human review: Pending author review
### Background
<!-- How the feature or module this PR touches works today, in two to four
plain sentences, for a reviewer who doesn't know it. Link the user doc if there
is one. Leave the problem and the fix to Problem Summary. -->
### What problem does this PR solve?
<!-- Issue Number: "close #N" if this PR resolves the issue, "#N" if it only
relates to it, "None" if there is none. A Feature needs an issue. -->
<!-- Related PR: "#N (relation)" for each PR a reviewer should know about,
such as "#123 (introduced the bug)", separated by commas. Write "None" if no
related PR is known. -->
<!-- PR Type: Bugfix, Performance, Feature, Enhancement, Refactor, Test,
Build/CI/Chore, Revert, Release/Dependency, or Other. -->
Issue Number:
Related PR:
PR Type:
Problem Summary:
<!-- Two to five sentences: the problem or need and who is affected; the
approach in one sentence; when the diff is larger than that suggests, what else
it had to handle, or that most of it is tests; and what existing users see
after an upgrade. -->
### Details
<!--
For a diff of more than about 300 changed lines, start with "- **Change
map:**": how many lines are production code, tests, generated files, and docs,
then the code in parts, each with the problem it handles and its size.
Then answer each field of your PR type on its own line, as "- **Field:**
answer", in this order:
Bugfix: Frequency, Root cause, Why existing tests missed it, Fix, Other
paths and branches
Performance: Why it was slow, Optimization, Results, Applicability and cost
Feature: Scope, Design, Usage, Default behavior, Compatibility
Enhancement: Before and after, Configs and defaults, Impact on existing
users
Refactor: Moves vs. logic changes, Behavior change, Compatibility,
Performance impact, Rollback
Test, adding tests: Scenarios, What existing tests miss, Fails when broken
Test, fixing a test: Failure, Cause, Stability evidence
Build/CI/Chore: Affected, Developer and release impact, License and
third-party, Rollback
Revert: Why not fix forward, Original author notified, Follow-up
Release/Dependency: Versions, Upstream changes, Compatibility, Rollback
Other: Why Other
"None" is a valid answer when it is true. Don't restate the diff.
-->
### Testing
<!-- The tests, suites, or commands that ran, what each shows, and where it
ran (locally or in CI). For a fix, the test that fails before it and passes
after. -->
### Notes for Reviewers
<!-- Optional; delete this section if there is nothing to say. Why each
non-obvious edit was made, what you are unsure about, and where to start
reading if Details has no Change map. -->
### Release note
<!-- One paragraph on what users will notice, or None. -->
### Check List (For Author)
<!-- Behavior changed: "No", or "Yes (see Details)" with a Behavior change
field in Details. -->
<!-- Does this need documentation: "No", or "Yes" with the
apache/doris-website PR or the plan. -->
<!-- Pre-submission status: "Ready"; "Partially verified: " and what is not
verified; or "Not ready: " and why, for a Draft PR. -->
- Behavior changed:
- Does this need documentation:
- Pre-submission status:
### AI Disclosure
<!-- Tool: the agent and model, or None. Generated: the parts it wrote
(code, tests, this description), or None. Human review: what you reviewed of
the generated parts, or "Not applicable" without AI tools. You may add "-
Prompts:" to summarize how you directed the agent. -->
- Tool:
- Generated:
- Human review:
--
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]
