-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Relax supportsPrewhere check for StorageMerge #61091
Conversation
This is an automated comment for commit a458344 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
|
src/Storages/StorageMerge.cpp
Outdated
@@ -364,6 +365,18 @@ void StorageMerge::read( | |||
const size_t max_block_size, | |||
size_t num_streams) | |||
{ | |||
if (query_info.prewhere_info) | |||
{ | |||
auto storage = getFirstTable([](const auto & table) { return !table->supportsPrewhere(); }); |
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.
This line of code duplicates tableSupportsPrewhere
, let's unify.
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.
It's different. tableSupportsPrewhere
is used to check if optimize_move_to_prewhere
is allowed while this line of code is to check if explicit PREWHERE is valid. I'll remove tableSupportsPrewhere
and move the implementation into canMoveConditionsToPrewhere
instead.
Any news here? @amosbird some CI/CD failures seems related to the change. |
I'm not sure. Will rebase to see if that resolves the issues. |
@amosbird, this is 99% ready to merge. Any idea, why
? |
Dear @KochetovNicolai, this PR hasn't been updated for a while. You will be unassigned. Will you continue working on it? If so, please feel free to reassign yourself. |
No "Bugfix validation — New test(s) failed to reproduce a bug" anymore. |
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Relax
supportsPrewhere
check for StorageMerge. This fixes #61064. It was hardened unnecessarily in #60082.