Skip to content

Cherry-pick: Respect use_client_time_zone for string datetime literals parsed on the server#2047

Open
zvonand wants to merge 2 commits into
antalya-25.8from
releasy/port/pr-109051-a8a99c
Open

Cherry-pick: Respect use_client_time_zone for string datetime literals parsed on the server#2047
zvonand wants to merge 2 commits into
antalya-25.8from
releasy/port/pr-109051-a8a99c

Conversation

@zvonand

@zvonand zvonand commented Jul 15, 2026

Copy link
Copy Markdown
Member

Cherry-picked from ClickHouse#109051.

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):

Fixed use_client_time_zone being ignored for DateTime/DateTime64 string literals interpreted on the server (asynchronous INSERT, SELECT literals). The client now propagates its local time zone as session_timezone when use_client_time_zone is enabled, so server-side parsing matches the synchronous INSERT path (ClickHouse#109051 by @groeneai).

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)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

…se-client-time-zone-108038

Respect use_client_time_zone for string datetime literals parsed on the server
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

Workflow [PR], commit [d34aa79]

@zvonand

zvonand commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

@blau-ai

@blau-ai

blau-ai commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

CI triage for #2047

Verdict: not green. 1 failure is PR-caused, the rest are pre-existing / infra / flaky.

The one real, PR-caused failure is the new test this PR adds04401_async_insert_use_client_time_zone. Everything else (Grype, S3Export regression, DCO, the other stateless tests, the aggregate gates) is unrelated to a client-side timezone change.


🔴 PR-caused — 04401_async_insert_use_client_time_zone

Fails consistently: 50/50 runs in Stateless tests (amd_asan, flaky check), and it also shows up in the failed-test list of the regular stateless jobs (amd_binary, ParallelReplicas and amd_binary, old analyzer, DatabaseReplicated). Not flaky — deterministic.

Decisive diff (reference vs. actual):

 flag_sync            1500036000     <- sync rows OK
 set_sync             1500036000
-flag_async           1500036000     <- expected (client TZ)
+flag_async           1500003600     <- actual   (server TZ)
-reset_default_async  1500036000
+reset_default_async  1500003600
-set_async            1500036000
+set_async            1500003600
-settings_async       1500036000
+settings_async       1500003600
 reset_async matches server tz   1

Reading it: the literal 2017-07-14 05:40:00 under TZ=America/Hermosillo (UTC‑7, no DST) should map to 1500036000. The sync rows (parsed client-side) are correct. Only the async rows are wrong, all off by exactly 9h (1500036000 → 1500003600), i.e. 05:40:00 interpreted in the server timezone (~UTC+2 in this CI env) instead of the client's. So the session_timezone the fix seeds in ClientBase::processParsedSingleQuery is not reaching the server-side async-INSERT VALUES parse in the antalya‑25.8 build. The reset_async matches server tz = 1 invariant still holds, which confirms the propagation only misfires when the client zone should be applied.

Why this isn't a bad cherry-pick: the diff here is byte-for-byte identical to upstream ClickHouse/ClickHouse#109051, which is green upstream. The capture in Client::connect() is also correctly ordered — and with use_client_time_zone=1 the server-TZ switch (Client.cpp:563) is skipped entirely, so client_local_timezone should hold the client OS zone. The fix works for sync but not async here ⇒ this is a 25.8-vs-26.x runtime divergence in the async-insert path, not a merge/port defect.

Suggested next step (needs a build — I can't run ClickHouse in this container, so this is a lead, not a validated fix):

  1. Repro locally: TZ=America/Hermosillo clickhouse-client --use_client_time_zone=1 -q "INSERT INTO t SETTINGS async_insert=1, wait_for_async_insert=1 VALUES ('2017-07-14 05:40:00')" and check toUnixTimestamp — expect it to come out in server TZ.
  2. Confirm whether session_timezone is actually transmitted for that async insert — check system.query_log / system.asynchronous_insert_log Settings for the query. The fix sets it on client_context right after applySettingsFromServerIfNeeded(); verify the antalya‑25.8 async-insert send path (and the server-side AsynchronousInsertQueue VALUES parse) reads that context. In 26.x it does; in 25.8 it likely captures the timezone before session_timezone is applied.
  3. Strong possibility: antalya‑25.8 is missing a prerequisite upstream commit that Respect use_client_time_zone for string datetime literals parsed on the server ClickHouse/ClickHouse#109051's async propagation depends on (settings transmission / async-insert context handling). Worth diffing the async-insert setting-propagation code between the two branches before writing a 25.8-specific hook.

I have not pushed anything. Once we have a candidate fix (or decide to gate the async cases for 25.8), I can open a blau/* PR so CI validates it — just say the word, or tell me to commit directly to releasy/port/pr-109051-a8a99c.


🟡 Not PR-caused

Check Classification Evidence
DCO Housekeeping The port commit b0005d3 (a re-used upstream merge commit) has no Signed-off-by: trailer. Needs a sign-off/rebase or a maintainer override; not a code issue.
Grype Scan ×3 (keeper, server, server-alpine) + GrypeScanKeeper/GrypeScanServer jobs Infra / pre-existing CVE scans of the built Docker images. A client-side C++ timezone change cannot affect image contents; these fail branch-wide independent of this PR.
RegressionTestsRelease / S3Export (part) & Regression x86 s3_export_part (14/85 scenarios) Pre-existing / flaky infra Server-side S3 export against MinIO — unrelated to client literal parsing. Note the sibling regressions passed: Regression x86 swarms (1488 ok) and s3_export_partition (7/7 ok).
Other stateless failures besides 04401: 03321_clickhouse_local_initialization_not_too_slow_even_under_sanitizers (timing/perf), 01710_projection_additional_filters, 04299_constraint_subst_correlated_subquery_root, 04340_datalake_schema_deep_recursion, 04409/04410_explain_actions_secret_args Pre-existing / flaky on antalya‑25.8 None touch client timezones; this PR only changes programs/client/Client.cpp + src/Client/ClientBase.{cpp,h} + the new test.
FinishCIReport, PR Aggregate gates Red only because of the checks above; no independent error.

Bottom line: the port applied cleanly, but the feature it backports is not fully functional for async inserts on antalya‑25.8 — that's the only thing this PR needs to resolve before merge. The remaining red is noise you'd see on the base branch too.

🤖 Automated CI triage by @blau-ai. I can't build/run ClickHouse in-container; the async root cause above is a lead to validate on a build, not a tested conclusion.

…r-side DateTime parsing

Test 04401_async_insert_use_client_time_zone (from ClickHouse#109051) requires the
server to respect session_timezone when parsing DateTime text into columns
without an explicit timezone. Upstream has this since PR ClickHouse#100647 (26.4,
backported to 26.3/26.2) but stable-25.8 does not, so async inserts (TCP)
and all inserts over HTTP were parsed in the server timezone.

- DataTypeDateTime/DataTypeDateTime64: resolve the effective timezone
  (respects session_timezone via DateLUT::instance()) when creating the
  default serialization. Adapted from upstream doGetSerialization() to
  25.8's doGetDefaultSerialization() API (no serialization object pool).
- MutationsDateTimeLiteralVisitor: wrap DateTime string literals in
  mutation predicates/assignments with explicit toDateTime(..., tz) at
  ALTER time, since background mutation threads lack the session context.
  Adapted make_intrusive -> std::make_shared for 25.8 AST pointers.
- Tests from the upstream PR: 02737 update, 04056, 04057.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

3 participants