Skip to content

Fix wrong date data type inference in case of overflow after timezone adjustment#102674

Merged
Avogar merged 1 commit intomasterfrom
fix-bt-date-overflow
Apr 16, 2026
Merged

Fix wrong date data type inference in case of overflow after timezone adjustment#102674
Avogar merged 1 commit intomasterfrom
fix-bt-date-overflow

Conversation

@Avogar
Copy link
Copy Markdown
Member

@Avogar Avogar commented Apr 14, 2026

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix wrong date data type inference in case of overflow after timezone adjustment. Closes #102601

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 14, 2026

Workflow [PR], commit [ebe9141]

Summary:

job_name test_name status info comment
Integration tests (amd_asan_ubsan, db disk, old analyzer, 5/6) failure
test_backup_restore_on_cluster/test_cancel_backup.py::test_short_disconnection_doesnt_stop_restore FAIL cidb, issue ISSUE CREATED

AI Review

Summary

This PR fixes parseDateTimeBestEffortImpl so DateTime inference correctly rejects values that become out of UInt32 range after timezone adjustment, then falls back to DateTime64 inference. The added stateless test covers both upper/lower boundary overflows and in-range controls. High-level verdict: the fix is targeted, aligns with the reported bug, and does not introduce obvious correctness, safety, or performance regressions.

ClickHouse Rules
Item Status Notes
Deletion logging
Serialization versioning
Core-area scrutiny
No test removal
Experimental gate
No magic constants
Backward compatibility
SettingsChangesHistory.cpp
PR metadata quality
Safe rollout
Compilation time
No large/binary files
Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Apr 14, 2026
@SmitaRKulkarni SmitaRKulkarni self-assigned this Apr 14, 2026
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 14, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.10% 84.00% -0.10%
Functions 90.90% 90.90% +0.00%
Branches 76.60% 76.60% +0.00%

Changed lines: 100.00% (4/4) | lost baseline coverage: 5 line(s) · Uncovered code

Full report · Diff report

@Avogar Avogar added this pull request to the merge queue Apr 16, 2026
Merged via the queue into master with commit 657d31f Apr 16, 2026
159 of 161 checks passed
@Avogar Avogar deleted the fix-bt-date-overflow branch April 16, 2026 19:43
@clickgapai
Copy link
Copy Markdown
Contributor

Hi — this PR may need backporting to 26.3 (LTS), 26.2, 26.1, 25.8 (LTS), but no backport label was found.

Affected code: parseDateTimeBestEffortImpl post-timezone-adjustment range check in parseDateTimeBestEffort.cpp — core code present in all supported branches.

Why: The bug was introduced by commit 0bb076a ('Improve schema inference of date times', 2024-08-15), which added the strict parsing path with the pre-adjustment range check but no post-adjustment check. This commit was also backported to 24.8, so the bug predates all currently supported branches. The fix prevents wrong type inference (DateTime instead of DateTime64) and wrong values (wrap-around) for datetime strings near epoch boundaries with timezone offsets.

If this should be backported, consider adding pr-must-backport or a version-specific label (e.g. v26.3-must-backport). Ignore this if backporting is not applicable.

@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Apr 16, 2026
@Avogar Avogar added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Apr 17, 2026
robot-ch-test-poll2 added a commit that referenced this pull request Apr 17, 2026
Cherry pick #102674 to 25.8: Fix wrong date data type inference in case of overflow after timezone adjustment
robot-clickhouse added a commit that referenced this pull request Apr 17, 2026
robot-ch-test-poll2 added a commit that referenced this pull request Apr 17, 2026
Cherry pick #102674 to 26.1: Fix wrong date data type inference in case of overflow after timezone adjustment
robot-clickhouse added a commit that referenced this pull request Apr 17, 2026
robot-ch-test-poll2 added a commit that referenced this pull request Apr 17, 2026
Cherry pick #102674 to 26.2: Fix wrong date data type inference in case of overflow after timezone adjustment
robot-clickhouse added a commit that referenced this pull request Apr 17, 2026
robot-ch-test-poll2 added a commit that referenced this pull request Apr 17, 2026
Cherry pick #102674 to 26.3: Fix wrong date data type inference in case of overflow after timezone adjustment
robot-clickhouse added a commit that referenced this pull request Apr 17, 2026
@robot-ch-test-poll robot-ch-test-poll added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Apr 17, 2026
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Apr 17, 2026
clickhouse-gh Bot added a commit that referenced this pull request Apr 17, 2026
Backport #102674 to 26.1: Fix wrong date data type inference in case of overflow after timezone adjustment
clickhouse-gh Bot added a commit that referenced this pull request Apr 17, 2026
Backport #102674 to 26.2: Fix wrong date data type inference in case of overflow after timezone adjustment
Avogar added a commit that referenced this pull request Apr 20, 2026
Backport #102674 to 26.3: Fix wrong date data type inference in case of overflow after timezone adjustment
Avogar added a commit that referenced this pull request Apr 20, 2026
Backport #102674 to 25.8: Fix wrong date data type inference in case of overflow after timezone adjustment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-bugfix Pull request with bugfix, not backported by default pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing range check after adjust_time_zone in parseDateTimeBestEffortImpl causes wrong DateTime inference

6 participants