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
Crash in Engine Merge if Row Policy does not have expression #61971
Conversation
This is an automated comment for commit 158743f with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page
Successful checks
|
Attn @vitlibar Not yet sure that it is a proper way to fix the issue, digging further. |
src/Storages/StorageMerge.cpp
Outdated
@@ -583,7 +583,7 @@ std::vector<ReadFromMerge::ChildPlan> ReadFromMerge::createChildrenPlans(SelectQ | |||
database_name, | |||
table_name, | |||
RowPolicyFilterType::SELECT_FILTER); | |||
if (row_policy_filter_ptr) | |||
if (row_policy_filter_ptr && row_policy_filter_ptr->expression) |
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.
Can you add a comment somewhere about why the USING 1
statement results in expression = nullptr
?
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.
Hello @pufit , as far as I can see zero expression is a normal course of actions.
The obvious reason is optimization in FiltersMixer::getResult
if (tryGetLiteralBool(result.get(), value) && value)
result = nullptr; /// The condition is always true, no need to check it.
May be it the the only one, not sure.
It seems that instead of checking "row_policy_filter_ptr->expression" I should call RowPolicyFilter::empty()
which probably makes a comment you are requesting redundant.
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.
We have exactly the same
if (row_policy_filter && !row_policy_filter->empty())
in InterpreterSelectQuery ctor, so this check seems a common pattern.
Test failures look unrelated. |
…36661784cfb1a283a1364104443a7 Cherry pick #61971 to 24.1: Crash in Engine Merge if Row Policy does not have expression
…36661784cfb1a283a1364104443a7 Cherry pick #61971 to 24.2: Crash in Engine Merge if Row Policy does not have expression
…36661784cfb1a283a1364104443a7 Cherry pick #61971 to 24.3: Crash in Engine Merge if Row Policy does not have expression
Backport #61971 to 24.2: Crash in Engine Merge if Row Policy does not have expression
Backport #61971 to 24.1: Crash in Engine Merge if Row Policy does not have expression
… have expression (#62220) Co-authored-by: robot-clickhouse <robot-clickhouse@users.noreply.github.com>
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fixes Crash in Engine Merge if Row Policy does not have expression
Addresses #61949