From 3004dbbc78718dfc3fec2fe9195e50f8e9760d10 Mon Sep 17 00:00:00 2001 From: Pxl Date: Thu, 2 Nov 2023 14:12:22 +0800 Subject: [PATCH] =?UTF-8?q?[Bug](materialized-view)=20SelectMaterializedIn?= =?UTF-8?q?dexWithAggregate=20do=20not=20=E2=80=A6=20(#26192)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SelectMaterializedIndexWithAggregate do not change plan when match base index --- .../analysis/CreateMaterializedViewStmt.java | 3 ++ .../SelectMaterializedIndexWithAggregate.java | 40 +++++++++++++++++++ .../trees/plans/logical/LogicalOlapScan.java | 13 ++---- .../rules/rewrite/mv/SelectMvIndexTest.java | 17 +++----- .../rewrite/mv/SelectRollupIndexTest.java | 31 +++++--------- .../nereids/trees/plans/PlanToStringTest.java | 6 +-- .../testProjectionMV1/testProjectionMV1.out | 8 ++++ .../testProjectionMV1.groovy | 17 ++++++++ 8 files changed, 89 insertions(+), 46 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 5c3c117a71..c42f3734f3 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 @@ -298,6 +298,9 @@ public class CreateMaterializedViewStmt extends DdlStmt { if (tableRefList.size() != 1) { throw new AnalysisException("The materialized view only support one table in from clause."); } + if (!isReplay && tableRefList.get(0).hasExplicitAlias()) { + throw new AnalysisException("The materialized view not support table with alias."); + } TableName tableName = tableRefList.get(0).getName(); if (tableName == null) { throw new AnalysisException("table in from clause is invalid, please check if it's single table " diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java index 12fe7b75c9..4c3179c290 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java @@ -116,6 +116,10 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial agg.getGroupByExpressions(), new HashSet<>(agg.getExpressions())); + if (result.indexId == scan.getTable().getBaseIndexId()) { + return ctx.root; + } + LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); @@ -162,6 +166,10 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial requiredExpr ); + if (result.indexId == scan.getTable().getBaseIndexId()) { + return ctx.root; + } + LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); @@ -207,6 +215,10 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial collectRequireExprWithAggAndProject(agg.getExpressions(), project.getProjects()) ); + if (result.indexId == scan.getTable().getBaseIndexId()) { + return ctx.root; + } + LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); @@ -265,6 +277,10 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial requiredExpr ); + if (result.indexId == scan.getTable().getBaseIndexId()) { + return ctx.root; + } + LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); @@ -322,6 +338,10 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial requiredExpr ); + if (result.indexId == scan.getTable().getBaseIndexId()) { + return ctx.root; + } + LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); @@ -369,6 +389,10 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial nonVirtualGroupByExprs(agg), new HashSet<>(agg.getExpressions())); + if (result.indexId == scan.getTable().getBaseIndexId()) { + return ctx.root; + } + LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); @@ -422,6 +446,10 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial requiredExpr ); + if (result.indexId == scan.getTable().getBaseIndexId()) { + return ctx.root; + } + LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); @@ -474,6 +502,10 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial collectRequireExprWithAggAndProject(agg.getExpressions(), project.getProjects()) ); + if (result.indexId == scan.getTable().getBaseIndexId()) { + return ctx.root; + } + LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); @@ -539,6 +571,10 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial requiredExpr ); + if (result.indexId == scan.getTable().getBaseIndexId()) { + return ctx.root; + } + LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); @@ -605,6 +641,10 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial requiredExpr ); + if (result.indexId == scan.getTable().getBaseIndexId()) { + return ctx.root; + } + LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOlapScan.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOlapScan.java index 6916631898..6e4a0bc246 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOlapScan.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOlapScan.java @@ -178,12 +178,8 @@ public class LogicalOlapScan extends LogicalCatalogRelation implements OlapScan @Override public String toString() { - return Utils.toSqlString("LogicalOlapScan", - "qualified", qualifiedName(), - "indexName", getSelectedMaterializedIndexName().orElse(""), - "selectedIndexId", selectedIndexId, - "preAgg", preAggStatus - ); + return Utils.toSqlString("LogicalOlapScan", "qualified", qualifiedName(), "indexName", + getSelectedMaterializedIndexName(), "selectedIndexId", selectedIndexId, "preAgg", preAggStatus); } @Override @@ -291,9 +287,8 @@ public class LogicalOlapScan extends LogicalCatalogRelation implements OlapScan } @VisibleForTesting - public Optional getSelectedMaterializedIndexName() { - return indexSelected ? Optional.ofNullable(((OlapTable) table).getIndexNameById(selectedIndexId)) - : Optional.empty(); + public String getSelectedMaterializedIndexName() { + return ((OlapTable) table).getIndexNameById(selectedIndexId); } @Override diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMvIndexTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMvIndexTest.java index 104ff9e0ec..a31ec922cf 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMvIndexTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMvIndexTest.java @@ -875,18 +875,11 @@ class SelectMvIndexTest extends BaseMaterializedIndexSelectTest implements MemoP String query = "select * from (select user_id, bitmap_union_count(to_bitmap(tag_id)) x from " + USER_TAG_TABLE_NAME + " group by user_id) a, (select user_name, bitmap_union_count(to_bitmap(tag_id))" + "" + " y from " + USER_TAG_TABLE_NAME + " group by user_name) b where a.x=b.y;"; - PlanChecker.from(connectContext) - .analyze(query) - .rewrite() - .matches(logicalJoin( - logicalProject( - logicalAggregate( - logicalOlapScan().when(scan -> "user_tags_mv".equals( - scan.getSelectedMaterializedIndexName().get())))), - logicalAggregate( - logicalProject( - logicalOlapScan().when(scan -> "user_tags".equals( - scan.getSelectedMaterializedIndexName().get())))))); + PlanChecker.from(connectContext).analyze(query).rewrite().matches(logicalJoin( + logicalProject(logicalAggregate(logicalOlapScan() + .when(scan -> "user_tags_mv".equals(scan.getSelectedMaterializedIndexName())))), + logicalAggregate(logicalProject(logicalOlapScan() + .when(scan -> "user_tags".equals(scan.getSelectedMaterializedIndexName())))))); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectRollupIndexTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectRollupIndexTest.java index ed5a96933d..2340e5e226 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectRollupIndexTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectRollupIndexTest.java @@ -112,7 +112,7 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .applyTopDown(new SelectMaterializedIndexWithAggregate()) .matches(logicalOlapScan().when(scan -> { Assertions.assertTrue(scan.getPreAggStatus().isOn()); - Assertions.assertEquals("t", scan.getSelectedMaterializedIndexName().get()); + Assertions.assertEquals("t", scan.getSelectedMaterializedIndexName()); return true; })); } @@ -124,7 +124,7 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .applyTopDown(new SelectMaterializedIndexWithAggregate()) .matches(logicalOlapScan().when(scan -> { Assertions.assertTrue(scan.getPreAggStatus().isOn()); - Assertions.assertEquals("r2", scan.getSelectedMaterializedIndexName().get()); + Assertions.assertEquals("r2", scan.getSelectedMaterializedIndexName()); return true; })); } @@ -139,9 +139,6 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M public void testTranslateWhenPreAggIsOff() { singleTableTest("select k2, min(v1) from t group by k2", scan -> { Assertions.assertFalse(scan.isPreAggregation()); - Assertions.assertEquals("Aggregate operator don't match, " - + "aggregate function: min(v1), column aggregate type: SUM", - scan.getReasonOfPreAggregation()); }); } @@ -152,7 +149,7 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .applyTopDown(new SelectMaterializedIndexWithAggregate()) .matches(logicalOlapScan().when(scan -> { Assertions.assertTrue(scan.getPreAggStatus().isOn()); - Assertions.assertEquals("r2", scan.getSelectedMaterializedIndexName().get()); + Assertions.assertEquals("r2", scan.getSelectedMaterializedIndexName()); return true; })); } @@ -164,7 +161,7 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .applyTopDown(new SelectMaterializedIndexWithAggregate()) .matches(logicalOlapScan().when(scan -> { Assertions.assertTrue(scan.getPreAggStatus().isOn()); - Assertions.assertEquals("r2", scan.getSelectedMaterializedIndexName().get()); + Assertions.assertEquals("r2", scan.getSelectedMaterializedIndexName()); return true; })); } @@ -176,7 +173,7 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .applyTopDown(new SelectMaterializedIndexWithAggregate()) .matches(logicalOlapScan().when(scan -> { Assertions.assertTrue(scan.getPreAggStatus().isOn()); - Assertions.assertEquals("r1", scan.getSelectedMaterializedIndexName().get()); + Assertions.assertEquals("r1", scan.getSelectedMaterializedIndexName()); return true; })); } @@ -195,7 +192,7 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .applyTopDown(new SelectMaterializedIndexWithoutAggregate()) .matches(logicalOlapScan().when(scan -> { Assertions.assertTrue(scan.getPreAggStatus().isOn()); - Assertions.assertEquals("r2", scan.getSelectedMaterializedIndexName().get()); + Assertions.assertEquals("r2", scan.getSelectedMaterializedIndexName()); return true; })); } @@ -227,8 +224,6 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .matches(logicalOlapScan().when(scan -> { PreAggStatus preAgg = scan.getPreAggStatus(); Assertions.assertTrue(preAgg.isOff()); - Assertions.assertEquals("Aggregate operator don't match, " - + "aggregate function: min(v1), column aggregate type: SUM", preAgg.getOffReason()); return true; })); } @@ -242,8 +237,6 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .matches(logicalOlapScan().when(scan -> { PreAggStatus preAgg = scan.getPreAggStatus(); Assertions.assertTrue(preAgg.isOff()); - Assertions.assertEquals("Slot((v1 + 1)) in sum((v1 + 1)) is neither key column nor value column.", - preAgg.getOffReason()); return true; })); } @@ -257,8 +250,6 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .matches(logicalOlapScan().when(scan -> { PreAggStatus preAgg = scan.getPreAggStatus(); Assertions.assertTrue(preAgg.isOff()); - Assertions.assertEquals("Aggregate function sum(k2) contains key column k2.", - preAgg.getOffReason()); return true; })); } @@ -273,7 +264,7 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .matches(logicalOlapScan().when(scan -> { PreAggStatus preAgg = scan.getPreAggStatus(); Assertions.assertTrue(preAgg.isOn()); - Assertions.assertEquals("r4", scan.getSelectedMaterializedIndexName().get()); + Assertions.assertEquals("r4", scan.getSelectedMaterializedIndexName()); return true; })); } @@ -288,7 +279,7 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .matches(logicalOlapScan().when(scan -> { PreAggStatus preAgg = scan.getPreAggStatus(); Assertions.assertTrue(preAgg.isOn()); - Assertions.assertEquals("r4", scan.getSelectedMaterializedIndexName().get()); + Assertions.assertEquals("r4", scan.getSelectedMaterializedIndexName()); return true; })); } @@ -376,8 +367,6 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M public void testCountDistinctValueColumn() { singleTableTest("select k1, count(distinct v1) from t group by k1", scan -> { Assertions.assertFalse(scan.isPreAggregation()); - Assertions.assertEquals("Count distinct is only valid for key columns, but meet count(DISTINCT v1).", - scan.getReasonOfPreAggregation()); Assertions.assertEquals("t", scan.getSelectedIndexName()); }); } @@ -425,7 +414,7 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .rewrite() .matches(logicalOlapScan().when(scan -> { Assertions.assertTrue(scan.getHints().isEmpty()); - Assertions.assertEquals("r1", scan.getSelectedMaterializedIndexName().get()); + Assertions.assertEquals("r1", scan.getSelectedMaterializedIndexName()); PreAggStatus preAggStatus = scan.getPreAggStatus(); Assertions.assertTrue(preAggStatus.isOff()); Assertions.assertEquals("No aggregate on scan.", preAggStatus.getOffReason()); @@ -444,7 +433,7 @@ class SelectRollupIndexTest extends BaseMaterializedIndexSelectTest implements M .matches(logicalOlapScan().when(scan -> { Assertions.assertEquals(1, scan.getHints().size()); Assertions.assertEquals("PREAGGOPEN", scan.getHints().get(0)); - Assertions.assertEquals("r1", scan.getSelectedMaterializedIndexName().get()); + Assertions.assertEquals("r1", scan.getSelectedMaterializedIndexName()); PreAggStatus preAggStatus = scan.getPreAggStatus(); Assertions.assertTrue(preAggStatus.isOn()); return true; diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/PlanToStringTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/PlanToStringTest.java index 53c938d849..c1153b291b 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/PlanToStringTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/PlanToStringTest.java @@ -80,10 +80,8 @@ public class PlanToStringTest { @Test public void testLogicalOlapScan() { LogicalOlapScan plan = PlanConstructor.newLogicalOlapScan(0, "table", 0); - Assertions.assertTrue( - plan.toString().matches("LogicalOlapScan \\( qualified=db\\.table, " - + "indexName=, " - + "selectedIndexId=-1, preAgg=ON \\)")); + Assertions.assertTrue(plan.toString().matches("LogicalOlapScan \\( qualified=db\\.table, " + "indexName=table, " + + "selectedIndexId=-1, preAgg=ON \\)"), plan.toString()); } @Test diff --git a/regression-test/data/mv_p0/ut/testProjectionMV1/testProjectionMV1.out b/regression-test/data/mv_p0/ut/testProjectionMV1/testProjectionMV1.out index ba455b7e85..523176bb0a 100644 --- a/regression-test/data/mv_p0/ut/testProjectionMV1/testProjectionMV1.out +++ b/regression-test/data/mv_p0/ut/testProjectionMV1/testProjectionMV1.out @@ -9,3 +9,11 @@ 1 1 2 2 +-- !select_mv -- +1 2 +2 2 + +-- !select_mv -- +1 2 +2 2 + diff --git a/regression-test/suites/mv_p0/ut/testProjectionMV1/testProjectionMV1.groovy b/regression-test/suites/mv_p0/ut/testProjectionMV1/testProjectionMV1.groovy index c23b533832..329e54a1f2 100644 --- a/regression-test/suites/mv_p0/ut/testProjectionMV1/testProjectionMV1.groovy +++ b/regression-test/suites/mv_p0/ut/testProjectionMV1/testProjectionMV1.groovy @@ -34,6 +34,11 @@ suite ("testProjectionMV1") { sql """insert into emps values("2020-01-01",1,"a",1,1,1);""" sql """insert into emps values("2020-01-02",2,"b",2,2,2);""" + test { + sql "create materialized view emps_mv as select deptno, empid from emps t order by deptno;" + exception "errCode = 2," + } + createMV("create materialized view emps_mv as select deptno, empid from emps order by deptno;") sql """insert into emps values("2020-01-01",1,"a",1,1,1);""" @@ -50,4 +55,16 @@ suite ("testProjectionMV1") { contains "(emps_mv)" } qt_select_mv "select empid, deptno from emps order by empid;" + + explain { + sql("select empid, sum(deptno) from emps group by empid order by empid;") + contains "(emps_mv)" + } + qt_select_mv "select empid, sum(deptno) from emps group by empid order by empid;" + + explain { + sql("select deptno, sum(empid) from emps group by deptno order by deptno;") + contains "(emps_mv)" + } + qt_select_mv "select deptno, sum(empid) from emps group by deptno order by deptno;" }