Skip to content

Backport #104296 to Stable 25.8 - Disable additional_table_filters with parallel replicas and analyzer and w/o query plan serialization#2065

Merged
zvonand merged 3 commits into
stable-25.8from
backports/25.8/104296
Jul 21, 2026
Merged

Backport #104296 to Stable 25.8 - Disable additional_table_filters with parallel replicas and analyzer and w/o query plan serialization#2065
zvonand merged 3 commits into
stable-25.8from
backports/25.8/104296

Conversation

@mkmkme

@mkmkme mkmkme commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Disable additional_table_filters with parallel replicas and analyzer and w/o query plan serialization. Previously it may lead to incorrect results.

Analyzer rewrites query when sending to remotes (parallel replicas followers) to prepend database, and this breaks the additional_table_filters. And there is no clean way to fix this w/o query plan serialization (serialize_query_plan=1). So it has been explicitly disabled (throw exception) for now.

Fixes #2054

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

Disable additional_table_filters with parallel replicas and analyzer and w/o query plan serialization (ClickHouse#104296 by @azat)

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful 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)
  • 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)

azat and others added 2 commits July 17, 2026 09:31
Disable additional_table_filters with parallel replicas and analyzer and w/o query plan serialization
25.8 is missing some of the settings that are used by the original PR.
25.8 is also missing the EXPLAIN-visualisation for parallel-replicas
remote step, so the last query in the test doesn't actually check
anything and is therefore dropped.
@mkmkme mkmkme added backport Backport 25.8 25.8 Altinity Stable stable-25.8 labels Jul 17, 2026
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown

Workflow [PR], commit [6877ef3]

@mkmkme mkmkme added the stable label Jul 17, 2026
@mkmkme

mkmkme commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

Seems like fast test is failing. Will investigate

@mkmkme
mkmkme marked this pull request as draft July 17, 2026 12:02
@zvonand

zvonand commented Jul 21, 2026

Copy link
Copy Markdown
Member

@blau-ai

@zvonand
zvonand marked this pull request as ready for review July 21, 2026 10:35
@blau-ai

blau-ai commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

CI triage for #2065

Verdict: 3 red checks — 0 caused by this PR. 1 pre-existing test/config incompatibility, 2 infra/flaky (already auto-classified "broken"), and 1 metadata (DCO). The PR's actual change (Planner.cpp guard + new 04207_pr_additional_filters) is green everywhere it ran — Fast test, all Build variants, the full Integration matrix, both Regression suites, and every other stateless job passed.


1. Stateless tests (amd_binary, ParallelReplicas, s3 storage, parallel)Failed: 1, Broken: 2NOT PR-caused

The 1 real failure: 04299_constraint_subst_correlated_subquery_root

Code: 48. DB::Exception: Cannot clone ReadFromPreparedSource plan step. (NOT_IMPLEMENTED)
  ...decorrelateQueryPlan (PlannerCorrelatedSubqueries.cpp:273)
  ...executeQueryWithParallelReplicas (ClusterProxy/executeQuery.cpp:771)
(query: SELECT count() FROM t_constraint_corr_root WHERE (b < d) OR (SELECT a < c)
        SETTINGS enable_analyzer=1, convert_query_to_cnf=1, optimize_substitute_columns=1, optimize_using_constraints=1)

This is the correlated-subquery decorrelation path, which has nothing to do with additional_table_filters. Evidence it isn't ours:

  • gh pr diff 2065 touches only src/Planner/Planner.cpp and the two 04207_pr_additional_filters.* files — it does not touch 04299.
  • 04299 already exists on stable-25.8 (last modified 2026-06-19, "Use MergeTree in 04299 constraint correlated subquery test"), well before this backport.
  • The test's own comment explains the root cause: it deliberately uses MergeTree because "a read step that cannot be cloned (e.g. TinyLog) makes correlated-subquery decorrelation fail with NOT_IMPLEMENTED... use MergeTree, whose read step is cloneable." Under the ParallelReplicas config the table read is replaced by a non-cloneable ReadFromPreparedSource, so the author's assumption breaks in this config only. It's a pre-existing test/config incompatibility surfaced by the ParallelReplicas job, not a regression from this PR.

The 2 "Broken" (non-blocking): 03441_deltalake_clickhouse_public_datasets, 03441_deltalake_clickhouse_virtual_columns — external S3 fetch timeouts:

Code: 742. DELTA_KERNEL_ERROR: ... Generic S3 error: Error performing GET
  https://s3.../clickhouse-public-datasets/delta_lake/hits/_delta_log/_last_checkpoint  after 10 retries
→ matched rule {'reason': 'INVESTIGATE - S3 sometimes unreachable'}

Pure infra flakiness (DeltaLake against a public S3 bucket); the framework already classified them as known-broken, so they don't block. Unrelated to this PR.

2. PRaggregate gate, NOT independent

No error of its own; it's red only because the stateless job above is red. Clears automatically once that job is green.

3. DCONOT PR-caused (metadata)

None of the 3 commits on backports/25.8/104296 carry a Signed-off-by trailer (adjust test for 25.8, tests: properly backport 04207 to 25.8, Merge pull request #104296...). This is a sign-off formality, independent of code correctness.


Suggested next steps

  • 04299 failure — nothing to fix in this PR. In practice the ParallelReplicas job applies parallel replicas non-deterministically, so a plain re-run may well come back green. The durable fix belongs in a separate change: tag the test -- Tags: no-parallel-replicas (or make the decorrelation clone the parallel-replicas read step). I can open that as a standalone blau/* PR against stable-25.8 if you'd like — just say the word.
  • DCO — re-create the commits with sign-off, e.g. git rebase --signoff stable-25.8 then push (I can't do this — it needs a force-push, which is outside my permissions, and the commits are yours/@mkmkme's). Altinity backport PRs often merge past DCO anyway; your call.
  • PR gate — resolves on its own once the stateless job is green.

Net: the code in this backport is not responsible for any red check. No change to the PR is warranted for correctness; the one real test failure is a pre-existing 04299-vs-ParallelReplicas incompatibility.

🤖 Generated with Claude Code · evidence from run 29763649169

@zvonand
zvonand merged commit ebed372 into stable-25.8 Jul 21, 2026
146 of 149 checks passed
@zvonand zvonand added the verified Approved for release label Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

25.8 25.8 Altinity Stable backport Backport stable stable-25.8 verified Approved for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants