-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Support create materialized view with bitmap or hll #3651
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
|
For Review , add UT later |
morningman
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.
And by the way, when the bitmap and hll types are not yet supported on the BE side, should these new operations be prohibited first?
fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
Outdated
Show resolved
Hide resolved
fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
Show resolved
Hide resolved
| Preconditions.checkState(functionCallExpr.getChildren().size() == 1); | ||
| Expr functionChild0 = functionCallExpr.getChild(0); | ||
|
|
||
| if (functionName.equalsIgnoreCase("bitmap_union") || functionName.equalsIgnoreCase("hll_union")) { |
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.
I think we need an extensible framework to handle the expression part of the materialized view more elegantly, rather than simply judging a few functions.
This can prepare for more complex expressions later.
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.
I agree with you. But schema change and expression compute logical is non-universal. so we can only support few function now.
| } else { | ||
| throw new DdlException("The define expr of column is only support bitmap_union or hll_union"); | ||
| } | ||
| newMVColumn.setIsKey(false); |
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.
The isKey has been set.
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 the aggregate function is computed for the original column, it should be not key whether the original column is a key column or a value column
fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
Outdated
Show resolved
Hide resolved
fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
Outdated
Show resolved
Hide resolved
fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
Outdated
Show resolved
Hide resolved
|
The commit msg is too short. |
morningman
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
For #3344