Skip to content

Fix unquoted timezone in sorting key#1526

Merged
zvonand merged 1 commit intoantalya-26.1from
bugfix/antalya-26.1/1487_fix_unquoted_timezone
Mar 24, 2026
Merged

Fix unquoted timezone in sorting key#1526
zvonand merged 1 commit intoantalya-26.1from
bugfix/antalya-26.1/1487_fix_unquoted_timezone

Conversation

@ianton-ru
Copy link
Copy Markdown

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 to CHANGELOG.md):

Solved #1487
Fix timezone parameter in sorting key for Iceberg table when type of key is DateTime and setting iceberg_partition_timezone is used.

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • Tiered Storage (2h)

@ianton-ru
Copy link
Copy Markdown
Author

@codex review

@github-actions
Copy link
Copy Markdown

Workflow [PR], commit [c116dc7]

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@CarlosFelipeOR
Copy link
Copy Markdown
Collaborator

QA Verification

PR Summary

This PR fixes issue #1487: when iceberg_partition_timezone is set and a DataLakeCatalog table has a temporal sort key (hour/day/month/year transform), the timezone was appended unquoted into the generated SQL expression — causing a parse error at query time.

Root cause: getSortingKeyDescriptionFromMetadata() in Utils.cpp built expressions like toRelativeDayNum(ts, UTC) (missing quotes) and toRelativeDayNum(ts, Asia / Istanbul) (slash treated as division). The fix wraps the timezone in single quotes.

Integration Tests (Altinity/ClickHouse CI)

CI run #23058230659

PR test passed:

  • test_database_iceberg/test_partition_timezone.py::test_partition_timezonePASSED in Integration tests (amd_asan, db disk, old analyzer, 2/6) (job 66996560914)

Note: The Integration tests (amd_asan, targeted) job (2m6s) ended with skipped [] | No failed tests found from previous runs — this is expected CI behavior (targeted job only retries previously failed tests), not a test failure.

Unrelated failures:

Test / Job Status Root Cause
Integration tests (amd_asan, db disk, old analyzer, 4/6) 5 ERRORs test_ldap_external_user_directory — setup errors (Docker container issue); all 5 passed on retry in 39s. Unrelated to Iceberg.
Stateless tests (arm_asan, targeted) 2 FAILs 01171_mv_select_insert_isolation_long (530s) and 02883_zookeeper_finalize_stress (248s) — long-running stress tests that time out on slow ARM ASAN runners. Unrelated to Iceberg.

Local Verification

Tested against the binary before the fix and after:

Before fix (unquoted timezone):

  • DayTransform + UTC: DB::Exception: Cannot parse expression of type ... token UTC
  • DayTransform + Asia/Istanbul: DB::Exception: Syntax error: ... unexpected token /

After fix (PR #1526 binary): All 7 scenarios pass — DayTransform, HourTransform, MonthTransform, YearTransform with UTC; DayTransform with Asia/Istanbul (positive offset) and America/New_York (negative offset); DayTransform on TimestamptzType.

Regression Tests (Altinity/clickhouse-regression)

Regression tests for this fix are being added in Altinity/clickhouse-regression (PR pending) at iceberg/tests/iceberg_engine/sort_key_timezone.py.

Coverage compared to the upstream integration test:

Scenario Upstream test_partition_timezone New regression test
DayTransform + UTC
HourTransform + UTC
MonthTransform + UTC
YearTransform + UTC
DayTransform + positive offset (Asia/Istanbul)
DayTransform + negative offset (America/New_York)
DayTransform + TimestamptzType

The regression tests are skipped on non-antalya builds and on versions < 26.1 (where iceberg_partition_timezone sort key support was introduced).

Conclusion

test_partition_timezone passed in CI. All other failures are demonstrably unrelated to the PR changes. The fix is confirmed by local testing before and after the patch. Approved.

@CarlosFelipeOR
Copy link
Copy Markdown
Collaborator

AI audit note: This review comment was generated by AI (gpt-5.3-codex).

Audit update for PR #1526 (Fix unquoted timezone in sorting key):

Confirmed defects:

  • No confirmed defects in reviewed scope.

Coverage summary:

  • Scope reviewed: src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp sorting-key expression construction path, related transform/timezone plumbing (parseTransformAndArgument, SettingFieldTimezone validation), and the PR test update in tests/integration/test_database_iceberg/test_partition_timezone.py.
  • Categories failed: none.
  • Categories passed: call-graph/transition consistency, timezone validation + literalization path, branch outcomes (identity vs transformed key; timezone present vs empty), error-contract consistency for invalid timezone settings, C++ bug-class checks in changed path (lifetime, race/deadlock, exception-safety, UB/signedness) with no confirmed issues.
  • Assumptions/limits: static audit only (no runtime fault injection executed in this session); conclusions are limited to the PR diff and directly connected code paths.

@zvonand zvonand merged commit 60fe13a into antalya-26.1 Mar 24, 2026
436 of 443 checks passed
@svb-alt svb-alt added the port-antalya PRs to be ported to all new Antalya releases label Apr 2, 2026
ianton-ru pushed a commit that referenced this pull request Apr 13, 2026
…nquoted_timezone

Fix unquoted timezone in sorting key
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya antalya-26.1 antalya-26.1.6.20001 bugfix port-antalya PRs to be ported to all new Antalya releases verified Approved for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants