-
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
Fix early constant folding for isNull/isNotNul and analyzer. #64695
Conversation
This is an automated comment for commit 6baae1d 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/DataTypes/DataTypeNullable.cpp
Outdated
@@ -174,4 +175,17 @@ DataTypePtr removeNullableOrLowCardinalityNullable(const DataTypePtr & type) | |||
|
|||
} | |||
|
|||
bool canContainNull(const IDataType & type) | |||
{ |
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.
Strange formatting
if (const auto * variant = typeid_cast<const DataTypeVariant *>(&type)) | ||
for (const auto & elem : variant->getVariants()) | ||
if (canContainNull(*elem)) | ||
return true; |
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.
That's an interesting change. Is it because of distributed execution?
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.
No. Looks like variant just always can be nullable, this is implemented in selector.
@@ -539,8 +539,11 @@ PlannerExpressionsAnalysisResult buildExpressionAnalysisResult(const QueryTreeNo | |||
if (query_node.hasWhere()) | |||
{ | |||
where_analysis_result_optional = analyzeFilter(query_node.getWhere(), current_output_columns, planner_context, actions_chain); | |||
where_action_step_index_optional = actions_chain.getLastStepIndex(); | |||
current_output_columns = actions_chain.getLastStepAvailableOutputColumns(); | |||
if (where_analysis_result_optional) |
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.
Why can it be "empty"?
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 are returning std::nullopt if where condition is constant 1.
Backport #64695 to 24.5: Fix early constant folding for isNull/isNotNul and analyzer.
Backport #64695 to 24.4: Fix early constant folding for isNull/isNotNul and analyzer.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix removing the
WHERE
andPREWHERE
expressions, which are always true (for the new analyzer)Fixes #64575