Skip to content

system.query_condition_cache: Add field for plaintext condition#78671

Merged
rschu1ze merged 3 commits intoClickHouse:masterfrom
rschu1ze:qcc_plaintext_condition
Apr 11, 2025
Merged

system.query_condition_cache: Add field for plaintext condition#78671
rschu1ze merged 3 commits intoClickHouse:masterfrom
rschu1ze:qcc_plaintext_condition

Conversation

@rschu1ze
Copy link
Copy Markdown
Member

@rschu1ze rschu1ze commented Apr 4, 2025

It was hard to understand for some queries which filter conditions a hash in the query condition cache represented. Now storing the original filter condition explicitly.

Changelog category (leave one):

  • Improvement

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

Added field condition to system table system.query_condition_cache. It stores the plaintext condition whose hash is used as a key in the query condition cache.

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Apr 4, 2025

Workflow [PR], commit [e6a34c9]

@clickhouse-gh clickhouse-gh bot added the pr-improvement Pull request with some product improvements label Apr 4, 2025
@rschu1ze rschu1ze marked this pull request as ready for review April 6, 2025 14:38
@rschu1ze rschu1ze force-pushed the qcc_plaintext_condition branch from 2b3dda7 to 2b65a0e Compare April 6, 2025 14:38
@alexey-milovidov

This comment was marked as resolved.

@rschu1ze

This comment was marked as resolved.

@rschu1ze rschu1ze force-pushed the qcc_plaintext_condition branch from 0b805b1 to 0714b7f Compare April 8, 2025 13:25
@devcrafter devcrafter self-assigned this Apr 9, 2025
const auto & outputs = filter_actions_dag->getOutputs();

/// Restrict to the case that ActionsDAG has a single output. This isn't technically necessary but de-risks the
/// implementatino a lot while not losing much usefulness.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
/// implementatino a lot while not losing much usefulness.
/// implementation a lot while not losing much usefulness.

{"table_uuid", std::make_shared<DataTypeUUID>(), "The table UUID."},
{"part_name", std::make_shared<DataTypeString>(), "The part name."},
{"key_hash", std::make_shared<DataTypeUInt64>(), "The hash of the filter condition."},
{"condition", std::make_shared<DataTypeString>(), "The hashed filter condition. Only set if setting query_condition_cache_store_conditions_as_plaintext = true."},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
{"condition", std::make_shared<DataTypeString>(), "The hashed filter condition. Only set if setting query_condition_cache_store_conditions_as_plaintext = true."},
{"condition", std::make_shared<DataTypeString>(), "The filter condition. Only set if setting query_condition_cache_store_conditions_as_plaintext = true."},

@rschu1ze
Copy link
Copy Markdown
Member Author

@devcrafter Thanks. Will incorporate your comments as part of #77983

@rschu1ze rschu1ze added this pull request to the merge queue Apr 11, 2025
Merged via the queue into ClickHouse:master with commit e516076 Apr 11, 2025
116 of 121 checks passed
@rschu1ze rschu1ze deleted the qcc_plaintext_condition branch April 11, 2025 11:37
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Apr 11, 2025
@rschu1ze rschu1ze added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Apr 16, 2025
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-backports-created-cloud deprecated label, NOOP label Apr 16, 2025
robot-ch-test-poll added a commit that referenced this pull request Apr 17, 2025
Cherry pick #78671 to 25.4: `system.query_condition_cache`: Add field for plaintext condition
robot-clickhouse added a commit that referenced this pull request Apr 17, 2025
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Apr 17, 2025
alexey-milovidov added a commit that referenced this pull request Apr 17, 2025
Backport #78671 to 25.4: `system.query_condition_cache`: Add field for plaintext condition
@robot-clickhouse robot-clickhouse added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-backports-created-cloud deprecated label, NOOP pr-improvement Pull request with some product improvements pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR 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.

7 participants