Skip to content

Fix unquoted timezone in sorting key#1526

Open
ianton-ru wants to merge 1 commit intoantalya-26.1from
bugfix/antalya-26.1/1487_fix_unquoted_timezone
Open

Fix unquoted timezone in sorting key#1526
ianton-ru wants to merge 1 commit intoantalya-26.1from
bugfix/antalya-26.1/1487_fix_unquoted_timezone

Conversation

@ianton-ru
Copy link

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
Author

@codex review

@github-actions
Copy link

Workflow [PR], commit [c116dc7]

@chatgpt-codex-connector
Copy link

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
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants