Skip to content

Antalya 25.8 Backport of 7a5f464 - Fix null pointer dereference in executeQueryImpl when use_query_cache=1 and stage=QueryPlan - #2568

Merged
mkmkme merged 1 commit into
antalya-25.8from
backports/antalya-25.8/7a5f464
Oct 14, 2026
Merged

mkmkme merged 1 commit into
antalya-25.8from
backports/antalya-25.8/7a5f464

Conversation

@mkmkme

@mkmkme mkmkme commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

When remote() is used with a single address and serialize_query_plan=1, the remote node receives the query with stage=QueryPlan and query_kind=INITIAL_QUERY. In that path, executeQueryImpl skips parsing, leaving out_ast null. The can_use_query_result_cache check at line 1649 then dereferences the null out_ast before the null guard can fire.

Fix: add && out_ast before the as<ASTSelectQuery>() call. The same guard already exists on line 1683 for a nearby check.

Observed in stress test 01018_Distributed__shard_num with fuzz-injected use_query_cache=1:
https://altinity-build-artifacts.s3.amazonaws.com/PRs/1867/0f1a2bdd63003e2d54b41fcaf7d71c020de69cca/stress_test_arm_asan_s3/clickhouse-server.err.log

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

Fix a failure in tsan stress test

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

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

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • CAS (content-addressed storage; Antalya only)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • OAuth (5m)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

…che=1` and `stage=QueryPlan`

When `remote()` is used with a single address and `serialize_query_plan=1`,
the remote node receives the query with `stage=QueryPlan` and
`query_kind=INITIAL_QUERY`. In that path, `executeQueryImpl` skips
parsing, leaving `out_ast` null. The `can_use_query_result_cache` check
at line 1649 then dereferences the null `out_ast` before the null guard
can fire.

Fix: add `&& out_ast` before the `as<ASTSelectQuery>()` call. The same
guard already exists on line 1683 for a nearby check.

Observed in stress test `01018_Distributed__shard_num` with fuzz-injected
`use_query_cache=1`:
https://altinity-build-artifacts.s3.amazonaws.com/PRs/1867/0f1a2bdd63003e2d54b41fcaf7d71c020de69cca/stress_test_arm_asan_s3/clickhouse-server.err.log

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Workflow [PR], commit [e5dfc9e]

@CarlosFelipeOR

Copy link
Copy Markdown
Collaborator

PR verification

AI-assisted analysis confirms the code change:

  • Faithful backport of 7a5f4649fd9 (already on antalya-26.3 through Antalya 26.3: Bump to 26.3.12 #1867). The diff is the same 1-line change as the original commit.
  • It fixes the SIGSEGV (NULL out_ast in executeQueryImpl, executeQuery.cpp:1440) seen in Stress test (amd_tsan) on the v25.8.33.20002.altinityantalya release run (38595158551), triggered by 01018_Distributed__shard_num with fuzz-injected use_query_cache=1.
  • The rest of executeQueryImpl on 25.8 already handles a null out_ast, so no other changes are needed.

Adding the verified label. Final confirmation will come once the stress test passes in the release run.

@mkmkme
mkmkme merged commit 94e80fa into antalya-25.8 Oct 14, 2026
76 of 77 checks passed
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