morningman commented on a change in pull request #3651:
URL: https://github.com/apache/incubator-doris/pull/3651#discussion_r428657017
##########
File path: fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
##########
@@ -79,6 +80,9 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import static org.apache.doris.catalog.AggregateType.BITMAP_UNION;
Review comment:
better not using static import
##########
File path:
fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
##########
@@ -167,15 +167,25 @@ private void analyzeSelectClause() throws
AnalysisException {
} else if (selectListItem.getExpr() instanceof FunctionCallExpr) {
FunctionCallExpr functionCallExpr = (FunctionCallExpr)
selectListItem.getExpr();
String functionName =
functionCallExpr.getFnName().getFunction();
+ Expr defineExpr = null;
// TODO(ml): support REPLACE, REPLACE_IF_NOT_NULL only for
aggregate table, HLL_UNION, BITMAP_UNION
if (!functionName.equalsIgnoreCase("sum")
&& !functionName.equalsIgnoreCase("min")
- && !functionName.equalsIgnoreCase("max")) {
+ && !functionName.equalsIgnoreCase("max")
+ && !functionName.equalsIgnoreCase("bitmap_union")
+ && !functionName.equalsIgnoreCase("hll_union")) {
throw new AnalysisException("The materialized view only
support the sum, min and max aggregate "
Review comment:
Change the exception message.
##########
File path: fe/src/main/java/org/apache/doris/catalog/Column.java
##########
@@ -310,6 +333,50 @@ public static String removeNamePrefix(String colName) {
return colName;
}
+ public Expr getDefineExpr() {
+ return defineExpr;
+ }
+
+ public void setDefineExpr(Expr expr) {
+ defineExpr = expr;
+ if (expr != null) {
+ serializeDefineExpr(expr);
+ }
+ }
+
+ public void serializeDefineExpr(Expr defineExpr) {
+ defineExprString = "COLUMNS (" + name + "=" + defineExpr.toSql() + ")";
+ }
+
+ public void deserializeDefineExpr(String defineExprString) {
+ if (defineExprString.equals("")) {
+ defineExpr = null;
+ } else {
+ try {
+ SqlParser parser = new SqlParser(new SqlScanner(new
StringReader(defineExprString)));
+ ImportColumnsStmt columnsStmt = (ImportColumnsStmt)
SqlParserUtils.getFirstStmt(parser);
+
+ ConnectContext ctx = new ConnectContext(null);
+ ctx.setCluster(SystemInfoService.DEFAULT_CLUSTER);
+ ctx.setCurrentUserIdentity(UserIdentity.ROOT);
+ ctx.setQualifiedUser(PaloAuth.ROOT_USER);
+ ctx.setCatalog(Catalog.getCurrentCatalog());
+ ctx.setThreadLocalInfo();
+
+ Analyzer analyzer = new Analyzer(Catalog.getCurrentCatalog(),
ctx);
+ columnsStmt.analyze(analyzer);
+ defineExpr = columnsStmt.getColumns().get(0).getExpr();
+ } catch (Exception e) {
+ e.printStackTrace();
Review comment:
I think we should throw this exception to terminate the process.
This should not happen.
##########
File path: fe/src/main/java/org/apache/doris/catalog/Column.java
##########
@@ -416,6 +483,7 @@ public void write(DataOutput out) throws IOException {
stats.write(out);
Text.writeString(out, comment);
+ Text.writeString(out, defineExprString);
Review comment:
This class `Column` is already adopt the GSON serialization. So I think
it can be completely converted to GSON this time.
##########
File path:
fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
##########
@@ -167,15 +167,25 @@ private void analyzeSelectClause() throws
AnalysisException {
} else if (selectListItem.getExpr() instanceof FunctionCallExpr) {
FunctionCallExpr functionCallExpr = (FunctionCallExpr)
selectListItem.getExpr();
String functionName =
functionCallExpr.getFnName().getFunction();
+ Expr defineExpr = null;
// TODO(ml): support REPLACE, REPLACE_IF_NOT_NULL only for
aggregate table, HLL_UNION, BITMAP_UNION
if (!functionName.equalsIgnoreCase("sum")
&& !functionName.equalsIgnoreCase("min")
- && !functionName.equalsIgnoreCase("max")) {
+ && !functionName.equalsIgnoreCase("max")
+ && !functionName.equalsIgnoreCase("bitmap_union")
+ && !functionName.equalsIgnoreCase("hll_union")) {
throw new AnalysisException("The materialized view only
support the sum, min and max aggregate "
+ "function. Error
function: " + functionCallExpr.toSqlImpl());
}
Preconditions.checkState(functionCallExpr.getChildren().size()
== 1);
Expr functionChild0 = functionCallExpr.getChild(0);
+
+ if (functionName.equalsIgnoreCase("bitmap_union") ||
functionName.equalsIgnoreCase("hll_union")) {
Review comment:
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.
##########
File path: fe/src/main/java/org/apache/doris/catalog/Column.java
##########
@@ -310,6 +333,50 @@ public static String removeNamePrefix(String colName) {
return colName;
}
+ public Expr getDefineExpr() {
+ return defineExpr;
+ }
+
+ public void setDefineExpr(Expr expr) {
+ defineExpr = expr;
+ if (expr != null) {
+ serializeDefineExpr(expr);
+ }
+ }
+
+ public void serializeDefineExpr(Expr defineExpr) {
+ defineExprString = "COLUMNS (" + name + "=" + defineExpr.toSql() + ")";
Review comment:
Actually, `toSql()` is really not a safe way for serialization. It may
be change, or even can not restore the original statement, in case the expr may
be much complicate and being rewritten somehow.
I was thinking maybe a safer way is to save the entire `create view`
statement directly. We shall discuss this later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
