This is an automated email from the ASF dual-hosted git repository.
panxiaolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new ce3afe7f13 [Enchancement](Materialized-View) forbiden some case in
create mv with group by and fix select fail on g… (#16820)
ce3afe7f13 is described below
commit ce3afe7f13cb8bc00bae679b3e2f48d19e620c78
Author: Pxl <[email protected]>
AuthorDate: Mon Feb 20 13:04:50 2023 +0800
[Enchancement](Materialized-View) forbiden some case in create mv with
group by and fix select fail on g… (#16820)
1. forbiden some case in create mv with group by
select k1+1,sum(abs(k2+2)+k3+3) from d_table group by k1;
2. fix select fail on grouping column have diffrent expr with select list
create materialized view k1p2ap3psg as select k1+1,sum(abs(k2+2)+k3+3) from
d_table group by k1+1;
mysql [test]>explain select k1+1,sum(abs(k2+2)+k3+3) from d_table group by
k1;
ERROR 1105 (HY000): errCode = 2, detailMessage = select list expression not
produced by aggregation output (missing from GROUP BY clause?): `k1` + 1
---
.../doris/analysis/CreateMaterializedViewStmt.java | 42 ++++++++++++++++++++--
.../java/org/apache/doris/analysis/SelectStmt.java | 17 +++++++--
.../doris/rewrite/mvrewrite/ExprToSlotRefRule.java | 5 +++
.../rewrite/mvrewrite/MVSelectFailedException.java | 4 +--
.../analysis/CreateMaterializedViewStmtTest.java | 14 ++++++++
.../agg_have_dup_base/agg_have_dup_base.out | 3 ++
.../multi_slot_k1a2p2ap3ps.out | 5 +++
.../agg_have_dup_base/agg_have_dup_base.groovy | 6 ++++
.../multi_slot_k1a2p2ap3ps.groovy | 14 ++++++++
9 files changed, 103 insertions(+), 7 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
index 76c86c62b3..e6c5a00793 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
@@ -158,6 +158,7 @@ public class CreateMaterializedViewStmt extends DdlStmt {
+ selectStmt.getHavingPred().toSql());
}
analyzeOrderByClause();
+ analyzeGroupByClause();
if (selectStmt.getLimit() != -1) {
throw new AnalysisException("The limit clause is not supported in
add materialized view clause, expr:"
+ " limit " + selectStmt.getLimit());
@@ -239,6 +240,39 @@ public class CreateMaterializedViewStmt extends DdlStmt {
dbName = tableName.getDb();
}
+ private void analyzeGroupByClause() throws AnalysisException {
+ if (selectStmt.getGroupByClause() == null) {
+ return;
+ }
+ List<Expr> groupingExprs =
selectStmt.getGroupByClause().getGroupingExprs();
+ List<FunctionCallExpr> aggregateExprs =
selectStmt.getAggInfo().getAggregateExprs();
+ List<Expr> selectExprs = selectStmt.getSelectList().getExprs();
+ for (Expr expr : selectExprs) {
+ boolean match = false;
+ String lhs =
selectStmt.getExprFromAliasSMap(expr).toSqlWithoutTbl();
+ for (Expr groupExpr : groupingExprs) {
+ String rhs =
selectStmt.getExprFromAliasSMap(groupExpr).toSqlWithoutTbl();
+ if (lhs.equalsIgnoreCase(rhs)) {
+ match = true;
+ break;
+ }
+ }
+ if (!match) {
+ for (Expr groupExpr : aggregateExprs) {
+ String rhs =
selectStmt.getExprFromAliasSMap(groupExpr).toSqlWithoutTbl();
+ if (lhs.equalsIgnoreCase(rhs)) {
+ match = true;
+ break;
+ }
+ }
+ }
+
+ if (!match) {
+ throw new AnalysisException("The select expr " + lhs + " not
in grouping or aggregate columns");
+ }
+ }
+ }
+
private void analyzeOrderByClause() throws AnalysisException {
if (selectStmt.getOrderByElements() == null) {
supplyOrderColumn();
@@ -513,16 +547,20 @@ public class CreateMaterializedViewStmt extends DdlStmt {
return name;
}
+ private static boolean mvMatch(String name, String prefix) {
+ return MaterializedIndexMeta.normalizeName(name).startsWith(prefix);
+ }
+
public static boolean isMVColumn(String name) {
return isMVColumnAggregate(name) || isMVColumnNormal(name);
}
public static boolean isMVColumnAggregate(String name) {
- return name.startsWith(MATERIALIZED_VIEW_AGGREGATE_NAME_PREFIX);
+ return mvMatch(name, MATERIALIZED_VIEW_AGGREGATE_NAME_PREFIX);
}
public static boolean isMVColumnNormal(String name) {
- return name.startsWith(MATERIALIZED_VIEW_NAME_PREFIX);
+ return mvMatch(name, MATERIALIZED_VIEW_NAME_PREFIX);
}
@Override
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
index 091db335cd..9363abca26 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
@@ -46,6 +46,7 @@ import org.apache.doris.common.util.SqlUtils;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.rewrite.ExprRewriter;
+import org.apache.doris.rewrite.mvrewrite.MVSelectFailedException;
import org.apache.doris.thrift.TExprOpcode;
import com.google.common.base.Preconditions;
@@ -1491,9 +1492,19 @@ public class SelectStmt extends QueryStmt {
// check that all post-agg exprs point to agg output
for (int i = 0; i < selectList.getItems().size(); ++i) {
if (!resultExprs.get(i).isBoundByTupleIds(groupingByTupleIds)) {
- throw new AnalysisException(
- "select list expression not produced by aggregation
output " + "(missing from "
- + "GROUP BY clause?): " +
selectList.getItems().get(i).getExpr().toSql());
+ if
(CreateMaterializedViewStmt.isMVColumn(resultExprs.get(i).toSqlWithoutTbl())) {
+ List<TupleId> tupleIds = Lists.newArrayList();
+ List<SlotId> slotIds = Lists.newArrayList();
+ resultExprs.get(i).getIds(tupleIds, slotIds);
+ for (TupleId id : tupleIds) {
+ updateDisableTuplesMVRewriter(id);
+ }
+ throw new MVSelectFailedException("Materialized View
rewrite invalid");
+ } else {
+ throw new AnalysisException(
+ "select list expression not produced by
aggregation output " + "(missing from "
+ + "GROUP BY clause?): " +
selectList.getItems().get(i).getExpr().toSql());
+ }
}
}
if (orderByElements != null) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/rewrite/mvrewrite/ExprToSlotRefRule.java
b/fe/fe-core/src/main/java/org/apache/doris/rewrite/mvrewrite/ExprToSlotRefRule.java
index ea03d0af62..688600fbdc 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/rewrite/mvrewrite/ExprToSlotRefRule.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/rewrite/mvrewrite/ExprToSlotRefRule.java
@@ -259,6 +259,11 @@ public class ExprToSlotRefRule implements ExprRewriteRule {
if (mvColumn == null) {
return expr;
+ } else {
+ Expr rhs = mvColumn.getDefineExpr();
+ if (rhs == null || !rhs.getClass().equals(expr.getClass())) {
+ return expr;
+ }
}
return rewriteExpr(tableName, mvColumn, analyzer);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/rewrite/mvrewrite/MVSelectFailedException.java
b/fe/fe-core/src/main/java/org/apache/doris/rewrite/mvrewrite/MVSelectFailedException.java
index 286fc71067..4a355885a1 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/rewrite/mvrewrite/MVSelectFailedException.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/rewrite/mvrewrite/MVSelectFailedException.java
@@ -17,9 +17,9 @@
package org.apache.doris.rewrite.mvrewrite;
-import org.apache.doris.common.UserException;
+import org.apache.doris.common.AnalysisException;
-public class MVSelectFailedException extends UserException {
+public class MVSelectFailedException extends AnalysisException {
public MVSelectFailedException(String msg) {
super(msg);
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java
b/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java
index 4762b3c9fe..9652fbd239 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java
@@ -561,6 +561,8 @@ public class CreateMaterializedViewStmtTest {
result = columnName3;
slotRef4.toSql();
result = columnName4;
+ selectStmt.getGroupByClause();
+ result = null;
}
};
@@ -664,6 +666,8 @@ public class CreateMaterializedViewStmtTest {
result = 4;
selectStmt.getAggInfo(); // return null, so that the mv can be
a duplicate mv
result = null;
+ selectStmt.getGroupByClause();
+ result = null;
}
};
@@ -762,6 +766,8 @@ public class CreateMaterializedViewStmtTest {
result = true;
selectStmt.getAggInfo(); // return null, so that the mv can be
a duplicate mv
result = null;
+ selectStmt.getGroupByClause();
+ result = null;
}
};
@@ -860,6 +866,8 @@ public class CreateMaterializedViewStmtTest {
result = PrimitiveType.VARCHAR;
selectStmt.getAggInfo(); // return null, so that the mv can be
a duplicate mv
result = null;
+ selectStmt.getGroupByClause();
+ result = null;
}
};
@@ -964,6 +972,8 @@ public class CreateMaterializedViewStmtTest {
result = columnName1;
slotRef1.getType().getPrimitiveType();
result = PrimitiveType.VARCHAR;
+ selectStmt.getGroupByClause();
+ result = null;
}
};
@@ -1040,6 +1050,8 @@ public class CreateMaterializedViewStmtTest {
result = columnName1;
slotRef2.getColumnName();
result = columnName2;
+ selectStmt.getGroupByClause();
+ result = null;
}
};
@@ -1095,6 +1107,8 @@ public class CreateMaterializedViewStmtTest {
result = null;
selectStmt.getHavingPred();
result = null;
+ selectStmt.getGroupByClause();
+ result = null;
selectStmt.getLimit();
result = -1;
slotRef1.toSql();
diff --git
a/regression-test/data/materialized_view_p0/agg_have_dup_base/agg_have_dup_base.out
b/regression-test/data/materialized_view_p0/agg_have_dup_base/agg_have_dup_base.out
index 815c51b9c2..c59641f5cd 100644
---
a/regression-test/data/materialized_view_p0/agg_have_dup_base/agg_have_dup_base.out
+++
b/regression-test/data/materialized_view_p0/agg_have_dup_base/agg_have_dup_base.out
@@ -23,3 +23,6 @@
2 2
3 -3
+-- !select_mv --
+\N -4
+
diff --git
a/regression-test/data/materialized_view_p0/multi_slot_k1a2p2ap3ps/multi_slot_k1a2p2ap3ps.out
b/regression-test/data/materialized_view_p0/multi_slot_k1a2p2ap3ps/multi_slot_k1a2p2ap3ps.out
index 5df5415679..a3c0a07a99 100644
---
a/regression-test/data/materialized_view_p0/multi_slot_k1a2p2ap3ps/multi_slot_k1a2p2ap3ps.out
+++
b/regression-test/data/materialized_view_p0/multi_slot_k1a2p2ap3ps/multi_slot_k1a2p2ap3ps.out
@@ -10,3 +10,8 @@
3 7
5 9
+-- !select_base --
+1 1
+3 7
+5 9
+
diff --git
a/regression-test/suites/materialized_view_p0/agg_have_dup_base/agg_have_dup_base.groovy
b/regression-test/suites/materialized_view_p0/agg_have_dup_base/agg_have_dup_base.groovy
index 452435a077..df0e50cac9 100644
---
a/regression-test/suites/materialized_view_p0/agg_have_dup_base/agg_have_dup_base.groovy
+++
b/regression-test/suites/materialized_view_p0/agg_have_dup_base/agg_have_dup_base.groovy
@@ -69,4 +69,10 @@ suite ("agg_have_dup_base") {
contains "(k12s3m)"
}
qt_select_mv "select k1,max(k2) from d_table group by k1 order by k1;"
+
+ explain {
+ sql("select unix_timestamp(k1) tmp,sum(k2) from d_table group by tmp;")
+ contains "(k12s3m)"
+ }
+ qt_select_mv "select unix_timestamp(k1) tmp,sum(k2) from d_table group by
tmp order by tmp;"
}
diff --git
a/regression-test/suites/materialized_view_p0/multi_slot_k1a2p2ap3ps/multi_slot_k1a2p2ap3ps.groovy
b/regression-test/suites/materialized_view_p0/multi_slot_k1a2p2ap3ps/multi_slot_k1a2p2ap3ps.groovy
index fb5f458f14..419ff5f42d 100644
---
a/regression-test/suites/materialized_view_p0/multi_slot_k1a2p2ap3ps/multi_slot_k1a2p2ap3ps.groovy
+++
b/regression-test/suites/materialized_view_p0/multi_slot_k1a2p2ap3ps/multi_slot_k1a2p2ap3ps.groovy
@@ -36,6 +36,14 @@ suite ("multi_slot_k1a2p2ap3ps") {
sql "insert into d_table select 2,2,2,'b';"
sql "insert into d_table select 3,-3,null,'c';"
+ boolean createFail = false;
+ try {
+ sql "create materialized view k1a2p2ap3ps as select
abs(k1)+k2+1,sum(abs(k2+2)+k3+3) from d_table group by abs(k1)+k2;"
+ } catch (Exception e) {
+ createFail = true;
+ }
+ assertTrue(createFail);
+
def result = "null"
sql "create materialized view k1a2p2ap3ps as select
abs(k1)+k2+1,sum(abs(k2+2)+k3+3) from d_table group by abs(k1)+k2+1;"
while (!result.contains("FINISHED")){
@@ -57,4 +65,10 @@ suite ("multi_slot_k1a2p2ap3ps") {
contains "(k1a2p2ap3ps)"
}
qt_select_mv "select abs(k1)+k2+1,sum(abs(k2+2)+k3+3) from d_table group
by abs(k1)+k2+1 order by abs(k1)+k2+1;"
+
+ explain {
+ sql("select abs(k1)+k2+1,sum(abs(k2+2)+k3+3) from d_table group by
abs(k1)+k2 order by abs(k1)+k2")
+ contains "(d_table)"
+ }
+ qt_select_base "select abs(k1)+k2+1,sum(abs(k2+2)+k3+3) from d_table group
by abs(k1)+k2 order by abs(k1)+k2;"
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
