Uh oh!
There was an error while loading. Please reload this page.
[fix](be) Normalize v1 date string cast result - #64575
Conversation
Problem Summary: Strict-mode string casts to v1 DATE can return early after parsing a compact datetime string or an hour-only time part. Those early returns skipped the common target-type normalization, so DATE results could keep non-zero time fields such as 2024-05-01 01:00:00 instead of 2024-05-01. Normalize the parsed value before both early returns so DATE clears the time fields and DATETIME keeps datetime semantics.
hello-stephen
commented
Jun 16, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
jacktengg
commented
Jun 16, 2026
/review |
jacktengg
commented
Jun 16, 2026
run buildall |
There was a problem hiding this comment.
Review result: requesting changes for a test-coverage gap.
Critical checkpoints:
- Goal: The PR intends to normalize v1 DATE string-cast results for compact datetime and hour-only strict-parser exits. The implementation now normalizes both changed exits, and non-strict callers also benefit because they delegate to this templated parser first.
- Scope: The implementation change is small and focused.
- Concurrency/lifecycle/config/compatibility/transactions/persistence/FE-BE protocol: Not involved in this PR.
- Parallel paths: Checked the v1 DATE/DATETIME serde callers and strict/non-strict parser flow; I did not find another implementation path needing the same normalization.
- Tests: The new BE unit test covers the hour-only v1 DATE case, but it does not cover the exact 14-digit YYYYMMDDHHMMSS branch, which is one of the two modified implementation branches.
- Test results: I did not run the BE unit test locally.
git diff --checkpassed. - Observability/performance/nullability: No new issue found; the change adds no new logging need, allocation path, or nullable-column behavior.
- User focus: No additional user-provided review focus was specified.
Uh oh!
There was an error while loading. Please reload this page.
hello-stephen
commented
Jun 16, 2026
TPC-H: Total hot run time: 28792 ms |
hello-stephen
commented
Jun 16, 2026
TPC-DS: Total hot run time: 175089 ms |
hello-stephen
commented
Jun 16, 2026
ClickBench: Total hot run time: 25.23 s |
hello-stephen
commented
Jun 16, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
PR approved by at least one committer and no changes requested. |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
jacktengg
commented
Jun 17, 2026
/review |
jacktengg
commented
Jun 17, 2026
run buildall |
There was a problem hiding this comment.
No blocking issues found in the current patch.
Checkpoint conclusions:
- Goal/test: The change normalizes v1 DATE results before both affected early returns in CastToDateOrDatetime::from_string_strict_mode. The added BE unit test covers the hour-only path and the exact 14-digit YYYYMMDDHHMMSS path, including the case previously raised in the existing review thread.
- Scope: The implementation is small and focused: two normalization calls plus targeted v1 DATE strict-mode coverage.
- Parallel paths: The modified parser is the v1 DATE/DATETIME path. DATEV2/DATETIMEV2 have separate implementations; the v1 final path already normalizes, and the two early exits now match it.
- Concurrency/lifecycle/config/compatibility/data writes/persistence/FE-BE protocol: Not applicable for this local cast parser change.
- Error handling: Existing Status/strict-mode handling is unchanged; the added calls do not introduce silent continuation.
- Performance/observability: No meaningful regression or new observability need; this is a constant-time normalization on early return paths.
- Test results: I did not run BE UT locally. git diff --check passed for the PR files. The visible macOS BE UT failure is a runner setup issue (Java 25 reported where JDK 17 is required) before tests compile/run, not a failure of this patch.
- User focus: No additional user-provided review focus was specified.
hello-stephen
commented
Jun 17, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Jun 17, 2026
TPC-H: Total hot run time: 29169 ms |
hello-stephen
commented
Jun 17, 2026
TPC-DS: Total hot run time: 176147 ms |
hello-stephen
commented
Jun 17, 2026
ClickBench: Total hot run time: 25.11 s |
hello-stephen
commented
Jun 17, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Uh oh!
There was an error while loading. Please reload this page.
Problem Summary: Strict-mode string casts to v1 DATE can return early after parsing a compact datetime string or an hour-only time part. Those early returns skipped the common target-type normalization, so DATE results could keep non-zero time fields such as 2024-05-01 01:00:00 instead of 2024-05-01. Normalize the parsed value before both early returns so DATE clears the time fields and DATETIME keeps datetime semantics. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Problem Summary: Strict-mode string casts to v1 DATE can return early after parsing a compact datetime string or an hour-only time part. Those early returns skipped the common target-type normalization, so DATE results could keep non-zero time fields such as 2024-05-01 01:00:00 instead of 2024-05-01. Normalize the parsed value before both early returns so DATE clears the time fields and DATETIME keeps datetime semantics.
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)