-
Notifications
You must be signed in to change notification settings - Fork 7
25.3.8 Backport of #87303 - Fix condition not being moved to PREWHERE in case there is a row policy (version 2) #1129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This commit is a manual backport of two PRs:
1. ClickHouse#87303
2. ClickHouse#88017
Additionally, the test from ClickHouse#88036
was added.
Also `02679_explain_merge_tree_prewhere_row_policy` was fixed as per
0aed477
| added = added || restoreDAGInputs(row_level_filter->actions, inputs); | ||
|
|
||
| added = added || restoreDAGInputs(info.prewhere_actions, inputs); | ||
| added = added || restoreDAGInputs(info->prewhere_actions, inputs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (info)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, great catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
ilejn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except minor issue I've commented
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test fails look suspicious. i.e. 02679_explain_merge_tree_prewhere_row_policy. Re-run did not help.
It seems I've forgotten to include the change to the test. We did change the reference in 24.8 to reflect this change. I'll fix it, thanks! |
This commit is a manual backport of two PRs:
1. ClickHouse#87303
2. ClickHouse#88017
Additionally, the test from ClickHouse#88036 was added.
Also
02679_explain_merge_tree_prewhere_row_policywas fixed as per 0aed477Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fixed move-to-prewhere optimization, which did not work in the presence of row policy (ClickHouse#87303 by @KochetovNicolai)
CI/CD Options
Exclude tests:
Regression jobs to run: