Skip to content

Fix skip index not used for ALIAS columns when query plan expression merging is disabled#98960

Merged
alexey-milovidov merged 2 commits intoClickHouse:masterfrom
fastio:bugfix-98822
Mar 9, 2026
Merged

Fix skip index not used for ALIAS columns when query plan expression merging is disabled#98960
alexey-milovidov merged 2 commits intoClickHouse:masterfrom
fastio:bugfix-98822

Conversation

@fastio
Copy link
Copy Markdown
Contributor

@fastio fastio commented Mar 7, 2026

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

Fix skip indexes (and primary key conditions) not being applied for ALIAS columns when query plan expression merging is disabled (query_plan_merge_expressions = 0
or query_plan_enable_optimizations = 0).

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

    When query plan expression merging is disabled, ExpressionStep nodes like "Compute alias columns" and "Change column names to column identifiers" remain separate
    from filter steps. The optimizePrimaryKeyConditionAndLimit optimization previously ignored these expression steps, causing filters on ALIAS columns to reference
    unresolved column identifiers that the storage layer could not match against indexes.

    This fix composes filter DAGs through accumulated expression DAGs (in bottom-to-top order) so that ALIAS column references are resolved to their underlying physical
    column expressions, enabling correct primary key and skip index analysis regardless of whether expression merging is active.

    Fixes Index using aliases does not work in new versions #98822


Note

Medium Risk
Touches query-plan optimization that builds primary key/skip index conditions; incorrect DAG composition could change which indexes are used or filter semantics for some plans. Covered by new regression tests, but the change impacts a performance-critical path.

Overview
Fixes index analysis when query_plan_merge_expressions=0 (or optimizations are disabled) by composing FilterStep DAGs through intervening ExpressionStep DAGs in optimizePrimaryKeyConditionAndLimit, so filters on ALIAS/renamed columns resolve to underlying physical expressions before being applied to storage.

Adds a stateless regression test (03822_alias_column_skip_index_no_merge) verifying EXPLAIN indexes=1 shows primary key and skip index conditions for ALIAS and nested-ALIAS predicates both with expression merging/optimizations disabled and under default settings.

Written by Cursor Bugbot for commit 4f99990. This will update automatically on new commits. Configure here.

@alexey-milovidov alexey-milovidov added the can be tested Allows running workflows for external contributors label Mar 7, 2026
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 7, 2026

Workflow [PR], commit [4f99990]

Summary:

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 7, 2026
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

But I found the idea of this PR controversial, because

when query plan expression merging is disabled (query_plan_merge_expressions = 0
or query_plan_enable_optimizations = 0).

who and why needs to disable this optimizations but still expect something to be optimized?

@alexey-milovidov
Copy link
Copy Markdown
Member

Should we still classify it as a "bug fix"? What the bug was?

@fastio
Copy link
Copy Markdown
Contributor Author

fastio commented Mar 7, 2026

Ok.

But I found the idea of this PR controversial, because

when query plan expression merging is disabled (query_plan_merge_expressions = 0
or query_plan_enable_optimizations = 0).

who and why needs to disable this optimizations but still expect something to be optimized?

Hi @alexey-milovidov, The real issue is #98822 — ALIAS column skip indexes stopped working after 24.2, which is a regression with default settings too. The disabled-optimizations scenario
is just the easiest way to reproduce and test it. The fix makes optimizePrimaryKeyConditionAndLimit not implicitly depend on mergeExpressions having run first, which is more robust.

@fastio
Copy link
Copy Markdown
Contributor Author

fastio commented Mar 7, 2026

Should we still classify it as a "bug fix"? What the bug was?

The bug is reported in #98822: skip indexes on ALIAS columns stopped working after version 24.2. Users observed that queries filtering on ALIAS columns no longer use
the skip index, leading to full granule scans. This is a user-visible regression in an official stable release, so "Bug Fix" seems appropriate.

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 9, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 83.20% 83.90% +0.70%
Functions 23.60% 23.90% +0.30%
Branches 75.30% 76.40% +1.10%

PR changed lines: PR changed-lines coverage: 100.00% (24/24)
Diff coverage report
Uncovered code

@alexey-milovidov alexey-milovidov merged commit 3a6ff77 into ClickHouse:master Mar 9, 2026
162 of 163 checks passed
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 9, 2026
mkmkme pushed a commit to Altinity/ClickHouse that referenced this pull request Mar 18, 2026
Fix skip index not used for ALIAS columns when query plan expression merging is disabled
zvonand added a commit to Altinity/ClickHouse that referenced this pull request Mar 20, 2026
24.8.14 Backport of ClickHouse#98960 - Fix skip index not used for ALIAS columns when query plan expression merging is disabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

can be tested Allows running workflows for external contributors pr-bugfix Pull request with bugfix, not backported by default pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Index using aliases does not work in new versions

4 participants