-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fix missing materialized column for hudi table #5119
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
| return scanColumns.size() != 0 && !((LogicalHiveScanOperator) scanOperator).getPartitionColumns().containsAll( | ||
| scanColumns.stream().map(ColumnRefOperator::getName).collect(Collectors.toList())); | ||
| } | ||
| if (scanOperator instanceof LogicalHudiScanOperator) { |
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 definitely need our DLA new framework to not let this duplicated code exist.
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.
Yes, it is necessary and more abstract operaters should be extracted to decouple from concrete table properties.
Make a rough refinement about this here, PTAL
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 definitely need our DLA new framework to not let this duplicated code exist.
that's true we should have a common prune rule which call api of connectors
|
run starrocks_fe_unittest |
1 similar comment
|
run starrocks_fe_unittest |
| private boolean containsMaterializedColumn(LogicalScanOperator scanOperator, Set<ColumnRefOperator> scanColumns) { | ||
| if (scanOperator instanceof LogicalHiveScanOperator) { | ||
| return scanColumns.size() != 0 && !((LogicalHiveScanOperator) scanOperator).getPartitionColumns().containsAll( | ||
| if (scanOperator instanceof LogicalHiveScanOperator || scanOperator instanceof LogicalHudiScanOperator) { |
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 need to add this check logic
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.
removed
| if (scanOperator instanceof LogicalHudiScanOperator) { | ||
| // Hudi partition columns is not materialized column, so except partition columns | ||
| return ((LogicalHudiScanOperator) scanOperator).getPartitionColumns().contains(columnName); | ||
| if (scanOperator instanceof LogicalHiveScanOperator || scanOperator instanceof LogicalHudiScanOperator) { |
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.
ditto
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.
removed
...e/src/main/java/com/starrocks/sql/optimizer/rule/transformation/PruneHDFSScanColumnRule.java
Show resolved
Hide resolved
caneGuy
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
|
@Mergifyio backport branch-2.2 |
(cherry picked from commit e12dea3)
✅ Backports have been createdDetails
|
[FE PR Coverage check]😍 pass : 7 / 7 (100.00%) file detail
|
(cherry picked from commit e12dea3) Co-authored-by: miomiocat <[email protected]>
Signed-off-by: EsoragotoSpirit <[email protected]> (cherry picked from commit 8c46037) Co-authored-by: 絵空事スピリット <[email protected]>
What type of PR is this:
Which issues of this PR fixes :
Closes #4753
Problem Summary(Required) :
Fix the wrong result when partition columns exists in
wherestatements