Skip to content

Fix ALIAS columns not applying timezone conversion for DateTime types in analyzer#101043

Merged
alexey-milovidov merged 2 commits intomasterfrom
fix-alias-datetime-timezone-cast
Mar 29, 2026
Merged

Fix ALIAS columns not applying timezone conversion for DateTime types in analyzer#101043
alexey-milovidov merged 2 commits intomasterfrom
fix-alias-datetime-timezone-cast

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

The analyzer used DataType::equals to check if a cast was needed for ALIAS columns, but DateTime and DateTime64 equals methods intentionally ignore timezone differences. This meant an alias like DateTime64(3, 'Europe/Berlin') ALIAS timestamp (where timestamp is DateTime64(3)) would skip the cast, returning the raw UTC value with just the timezone label changed — no actual timezone conversion.

Changed to getName() string comparison (matching the old interpreter behavior) so timezone differences are detected and a proper _CAST is applied.

Closes #76787

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 ALIAS columns with DateTime/DateTime64 types not applying timezone conversion when the declared timezone differs from the expression timezone.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

… in analyzer

The analyzer used `DataType::equals` to check if a cast was needed for ALIAS
columns, but `DateTime` and `DateTime64` `equals` methods intentionally ignore
timezone differences. This meant `DateTime64(3, 'Europe/Berlin') ALIAS timestamp`
(where `timestamp` is `DateTime64(3)`) would skip the cast, returning the raw UTC
value with just the timezone label changed.

Use `getName()` string comparison instead (matching the old interpreter behavior)
so timezone differences are detected and a proper `_CAST` is applied.

Closes #76787

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Mar 28, 2026

Workflow [PR], commit [a0b3a11]

Summary:


AI Review

Summary

This PR fixes a real analyzer correctness issue for ALIAS columns with DateTime/DateTime64 timezones by switching type-equality checks from equals (which intentionally ignores timezone) to getName-based comparison before applying _CAST. The change is small, targeted, and covered by a new stateless test that validates both DateTime64 and DateTime behavior. I did not find a high-confidence correctness, safety, or performance regression in the modified paths.

Missing context
  • ⚠️ CI had not completed at review time (only pending workflow status was visible), so this review is code-and-tests based.
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
Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 28, 2026
Copy link
Copy Markdown
Member Author

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix LGTM

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Mar 29, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.10% 84.20% +0.10%
Functions 24.50% 24.50% +0.00%
Branches 76.70% 76.80% +0.10%

Changed lines: 92.86% (13/14) · Uncovered code

Full report · Diff report

@alexey-milovidov alexey-milovidov self-assigned this Mar 29, 2026
@alexey-milovidov alexey-milovidov merged commit 573f827 into master Mar 29, 2026
152 of 153 checks passed
@alexey-milovidov alexey-milovidov deleted the fix-alias-datetime-timezone-cast branch March 29, 2026 15:30
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default 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.

Unexpected timezone handling with DateTime64 aliases in the analyzer

2 participants