Fix some cases where secondary index was not applied with analyzer. by KochetovNicolai · Pull Request #78485 · ClickHouse/ClickHouse

KochetovNicolai · 2025-03-31T17:12:16Z

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 some cases where secondary index was not applied with analyzer.
Fixes #65607 , fixes #69373

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

clickhouse-gh · 2025-03-31T17:12:43Z

Workflow [PR], commit [0c582bd]

devcrafter · 2025-04-11T12:01:14Z

tests/queries/0_stateless/03402_secondary_indexes_analyzer_bugs.sql

+
+INSERT INTO t SELECT toString(number), number, toString(number) from numbers(65536);
+
+explain indexes=1 select tenant,recordTimestamp from t where colAlias like '%abcd%' settings enable_analyzer=1;


(1) should we add check w/o analyzer?
(2) also, it's better to output only part of explain related to indices

devcrafter · 2025-04-11T12:03:37Z

tests/queries/0_stateless/03402_secondary_indexes_analyzer_bugs.sql

+
+--- #69373
+
+CREATE TABLE tab_v1


drop table if exists ... drop table ...

It's useful for running tests locally with default database

devcrafter · 2025-04-11T12:06:11Z

tests/queries/0_stateless/03402_secondary_indexes_analyzer_bugs.sql

+explain indexes=1 select tenant,recordTimestamp from t where colAlias like '%abcd%' settings enable_analyzer=1;
+
+
+--- #69373


Suggested change

--- #69373

-- #69373

select 'index is applied to view';

devcrafter · 2025-04-11T12:07:44Z

tests/queries/0_stateless/03402_secondary_indexes_analyzer_bugs.sql

@@ -0,0 +1,45 @@
+-- Tags: no-random-merge-tree-settings, no-random-settings, no-parallel-replicas
+
+--- #65607


Suggested change

--- #65607

-- #65607

select 'index is applied while using column alias';

devcrafter · 2025-04-11T12:23:11Z

src/Storages/MergeTree/KeyCondition.h

    /// Construct key condition from ActionsDAG nodes
    KeyCondition(
-        const ActionsDAG * filter_dag,
+        const ActionsDAGWithInversionPushDown & filter_dag,


Actually, we can construct ActionsDAGWithInversionPushDown inside KeyCondition constructor. context is already passed into KeyCondition as second parameter, just need to pass const ActionsDAG::Node * predicate_ as first parameter instead of const ActionsDAG * filter_dag. This way, we don't need to construct ActionsDAGWithInversionPushDown outside KeyCondition first.

The whole idea of adding the ActionsDAGWithInversionPushDown structure is to allow creating it outside of KeyCondition and make it reusable. We already did the same kind of work for PK and partition key, and now for secondary indexes too.

It's possible to keep an old ctor, but in this case it would be hard to check all the existing usages.

devcrafter · 2025-04-11T12:25:40Z

src/Storages/MergeTree/MergeTreeData.cpp

    ConditionSelectivityEstimator estimator;
-    PartitionPruner partition_pruner(storage_snapshot->metadata, filter_dag, local_context);
+    ActionsDAGWithInversionPushDown inverted_dag(filter_dag->getOutputs().front(), local_context);
+    PartitionPruner partition_pruner(storage_snapshot->metadata, inverted_dag, local_context);


Similar to KeyCondition, - ActionsDAGWithInversionPushDown can be constructed inside PartitionPruner

Fix rebase issue: - 20250502 ClickHouse#79180 - 20250416 ClickHouse#78485 - 20250306 ClickHouse#76662 Co-authored-by: liuneng1994 <neng.liu@kyligence.com>

Backport #78485 to 25.4: Fix some cases where secondary index was not applied with analyzer.

Fix rebase issue: - 20250502 ClickHouse#79180 - 20250416 ClickHouse#78485 - 20250306 ClickHouse#76662 Co-authored-by: liuneng1994 <neng.liu@kyligence.com>

Apply predicate canonisation to index condition.

09be62e

KochetovNicolai added 2 commits April 1, 2025 10:35

Merge branch 'master' into fix-some-secondary-index-for-analyzer

fb60204

Merge branch 'master' into fix-some-secondary-index-for-analyzer

a7e078c

clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Apr 1, 2025

KochetovNicolai added 2 commits April 1, 2025 15:33

Fixing tests.

11fe92f

Fixing build.

68f9134

devcrafter self-assigned this Apr 2, 2025

KochetovNicolai added 5 commits April 2, 2025 12:00

Fixing 02354_vector_search tests.

8e4ecef

Handling materialize. Added a test.

7e76201

Fixing fasttest.

59212c5

Merge branch 'master' into fix-some-secondary-index-for-analyzer

d801343

Fixing build.

c65df10

KochetovNicolai added pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-cloud labels Apr 2, 2025

KochetovNicolai added 10 commits April 3, 2025 10:38

Unflaky the new test.

bfb24f4

Remove randomness.

7ce795d

Merge branch 'master' into fix-some-secondary-index-for-analyzer

c7260ab

Fixing test.

1a951b3

Remove some unneeded code.

e94b97f

Update test result.

deb1157

Merge branch 'master' into fix-some-secondary-index-for-analyzer

c0f2568

Fixing build.

99c8141

Update PartitionPruner.cpp

a2d1a2b

Merge branch 'master' into fix-some-secondary-index-for-analyzer

a886fa8

devcrafter approved these changes Apr 11, 2025

View reviewed changes

KochetovNicolai added 3 commits April 14, 2025 12:46

Merge branch 'master' into fix-some-secondary-index-for-analyzer

2e69d0c

Update test.

4c6ccc8

Merge branch 'master' into fix-some-secondary-index-for-analyzer

0c582bd

KochetovNicolai enabled auto-merge April 15, 2025 10:24

KochetovNicolai mentioned this pull request May 9, 2025

bloom_filter not triggered for arrayMap(v -> lowerUTF8(v), arr) #79586

Closed

Algunenano added a commit that referenced this pull request May 22, 2025

Merge pull request #79892 from ClickHouse/backport/25.4/78485

86c5c00

Backport #78485 to 25.4: Fix some cases where secondary index was not applied with analyzer.

Avogar mentioned this pull request May 28, 2025

PipelineExecutor: Code: 49. DB::Exception: Too large size ... passed to allocator when trying to insert JSON data over HTTP #80727

Closed

alexey-milovidov mentioned this pull request Feb 6, 2026

Segmentation Fault in ClickHouse 25.3+ When Using Bloom Filter Indexes #81963

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix some cases where secondary index was not applied with analyzer.#78485

Fix some cases where secondary index was not applied with analyzer.#78485
KochetovNicolai merged 23 commits intomasterfrom
fix-some-secondary-index-for-analyzer

KochetovNicolai commented Mar 31, 2025 •

edited

Loading

Uh oh!

clickhouse-gh bot commented Mar 31, 2025 •

edited

Loading

Uh oh!

devcrafter Apr 11, 2025

Uh oh!

devcrafter Apr 11, 2025

Uh oh!

devcrafter Apr 11, 2025

Uh oh!

devcrafter Apr 11, 2025

Uh oh!

devcrafter Apr 11, 2025

Uh oh!

KochetovNicolai Apr 14, 2025

Uh oh!

devcrafter Apr 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants


		INSERT INTO t SELECT toString(number), number, toString(number) from numbers(65536);

		explain indexes=1 select tenant,recordTimestamp from t where colAlias like '%abcd%' settings enable_analyzer=1;

		explain indexes=1 select tenant,recordTimestamp from t where colAlias like '%abcd%' settings enable_analyzer=1;


		--- #69373

		@@ -0,0 +1,45 @@
		-- Tags: no-random-merge-tree-settings, no-random-settings, no-parallel-replicas

		--- #65607

	--- #65607
	-- #65607
	select 'index is applied while using column alias';

Conversation

KochetovNicolai commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Documentation entry for user-facing changes

Uh oh!

clickhouse-gh bot commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devcrafter Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

devcrafter Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

devcrafter Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

devcrafter Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

devcrafter Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

KochetovNicolai Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

devcrafter Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

KochetovNicolai commented Mar 31, 2025 •

edited

Loading

clickhouse-gh bot commented Mar 31, 2025 •

edited

Loading