From c0282bbc58f3ffb4481ebcfb0bae61c7e2614925 Mon Sep 17 00:00:00 2001 From: EmmyMiao87 <522274284@qq.com> Date: Wed, 25 Mar 2020 20:42:39 +0800 Subject: [PATCH] Solve the problem of mv selector when there is having clause in query (#3176) All of columns which belong to top of tupleIds in query should be considered in mv selector. For example: `select k1 from table group by k1 having sum(v1) >1;` The candidate index should contain k1 and v1 columns instead of only k1. The rollup which only has k1 column should not be selected. The issue #3174 describe in detail. --- .../doris/analysis/TupleDescriptor.java | 28 +++++++++++ .../planner/MaterializedViewSelector.java | 12 ++++- .../planner/MaterializedViewFunctionTest.java | 46 +++++++++++++++++-- .../planner/MaterializedViewSelectorTest.java | 32 ------------- 4 files changed, 81 insertions(+), 37 deletions(-) diff --git a/fe/src/main/java/org/apache/doris/analysis/TupleDescriptor.java b/fe/src/main/java/org/apache/doris/analysis/TupleDescriptor.java index dd80deb67c..2430c5e9df 100644 --- a/fe/src/main/java/org/apache/doris/analysis/TupleDescriptor.java +++ b/fe/src/main/java/org/apache/doris/analysis/TupleDescriptor.java @@ -32,6 +32,9 @@ import org.apache.logging.log4j.Logger; import java.util.ArrayList; import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; public class TupleDescriptor { private static final Logger LOG = LogManager.getLogger(TupleDescriptor.class); @@ -256,6 +259,31 @@ public class TupleDescriptor { for (SlotDescriptor slot: slots) slot.setIsMaterialized(true); } + public void getTableNameToColumnNames(Map> tupleDescToColumnNames) { + for (SlotDescriptor slotDescriptor : slots) { + if (!slotDescriptor.isMaterialized()) { + continue; + } + if (slotDescriptor.getColumn() != null) { + TupleDescriptor parent = slotDescriptor.getParent(); + Preconditions.checkState(parent != null); + Table table = parent.getTable(); + Preconditions.checkState(table != null); + String tableName = table.getName(); + Set columnNames = tupleDescToColumnNames.get(tableName); + if (columnNames == null) { + columnNames = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + tupleDescToColumnNames.put(tableName, columnNames); + } + columnNames.add(slotDescriptor.getColumn().getName()); + } else { + for (Expr expr : slotDescriptor.getSourceExprs()) { + expr.getTableNameToColumnNames(tupleDescToColumnNames); + } + } + } + } + @Override public String toString() { String tblStr = (table == null ? "null" : table.getName()); diff --git a/fe/src/main/java/org/apache/doris/planner/MaterializedViewSelector.java b/fe/src/main/java/org/apache/doris/planner/MaterializedViewSelector.java index 4cfdf5a322..dcfe276916 100644 --- a/fe/src/main/java/org/apache/doris/planner/MaterializedViewSelector.java +++ b/fe/src/main/java/org/apache/doris/planner/MaterializedViewSelector.java @@ -25,6 +25,8 @@ import org.apache.doris.analysis.FunctionCallExpr; import org.apache.doris.analysis.SelectStmt; import org.apache.doris.analysis.SlotRef; import org.apache.doris.analysis.TableRef; +import org.apache.doris.analysis.TupleDescriptor; +import org.apache.doris.analysis.TupleId; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.KeysType; import org.apache.doris.catalog.MaterializedIndexMeta; @@ -42,6 +44,7 @@ import com.google.common.collect.Sets; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; import java.util.List; @@ -474,8 +477,13 @@ public class MaterializedViewSelector { } // Step4: compute the output column - for (Expr resultExpr : selectStmt.getResultExprs()) { - resultExpr.getTableNameToColumnNames(columnNamesInQueryOutput); + // ISSUE-3174: all of columns which belong to top tuple should be considered in selector. + ArrayList topTupleIds = Lists.newArrayList(); + selectStmt.getMaterializedTupleIds(topTupleIds); + for (TupleId tupleId : topTupleIds) { + TupleDescriptor tupleDescriptor = analyzer.getTupleDesc(tupleId); + tupleDescriptor.getTableNameToColumnNames(columnNamesInQueryOutput); + } } diff --git a/fe/src/test/java/org/apache/doris/planner/MaterializedViewFunctionTest.java b/fe/src/test/java/org/apache/doris/planner/MaterializedViewFunctionTest.java index 43be8db71c..6a8351f32f 100644 --- a/fe/src/test/java/org/apache/doris/planner/MaterializedViewFunctionTest.java +++ b/fe/src/test/java/org/apache/doris/planner/MaterializedViewFunctionTest.java @@ -21,15 +21,12 @@ import org.apache.doris.common.FeConstants; import org.apache.doris.utframe.DorisAssert; import org.apache.doris.utframe.UtFrameUtils; -import org.apache.commons.io.FileUtils; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; -import java.io.File; -import java.io.IOException; import java.util.UUID; public class MaterializedViewFunctionTest { @@ -45,6 +42,7 @@ public class MaterializedViewFunctionTest { private static final String DEPTS_MV_NAME = "depts_mv"; private static final String QUERY_USE_DEPTS_MV = "rollup: " + DEPTS_MV_NAME; private static final String QUERY_USE_DEPTS = "rollup: " + DEPTS_TABLE_NAME; + private static final String TEST_TABLE_NAME = "test_tb"; private static DorisAssert dorisAssert; @BeforeClass @@ -584,4 +582,46 @@ public class MaterializedViewFunctionTest { String query = "select k1, k2 from agg_table;"; dorisAssert.withRollup(createRollupSQL).query(query).explainContains("OFF", "old_key"); } + + @Test + public void testAggFunctionInHaving() throws Exception { + String duplicateTable = "CREATE TABLE " + TEST_TABLE_NAME + " ( k1 int(11) NOT NULL , k2 int(11) NOT NULL ," + + "v1 varchar(4096) NOT NULL, v2 float NOT NULL , v3 decimal(20, 7) NOT NULL ) ENGINE=OLAP " + + "DUPLICATE KEY( k1 , k2 ) DISTRIBUTED BY HASH( k1 , k2 ) BUCKETS 3 " + + "PROPERTIES ('replication_num' = '1'); "; + dorisAssert.withTable(duplicateTable); + String createK1K2MV = "create materialized view k1_k2 as select k1,k2 from " + TEST_TABLE_NAME + " group by " + + "k1,k2;"; + String query = "select k1 from " + TEST_TABLE_NAME + " group by k1 having max(v1) > 10;"; + dorisAssert.withMaterializedView(createK1K2MV).query(query).explainWithout("k1_k2"); + dorisAssert.dropTable(TEST_TABLE_NAME); + } + + @Test + public void testAggFunctionInOrder() throws Exception { + String duplicateTable = "CREATE TABLE " + TEST_TABLE_NAME + " ( k1 int(11) NOT NULL , k2 int(11) NOT NULL ," + + "v1 varchar(4096) NOT NULL, v2 float NOT NULL , v3 decimal(20, 7) NOT NULL ) ENGINE=OLAP " + + "DUPLICATE KEY( k1 , k2 ) DISTRIBUTED BY HASH( k1 , k2 ) BUCKETS 3 " + + "PROPERTIES ('replication_num' = '1'); "; + dorisAssert.withTable(duplicateTable); + String createK1K2MV = "create materialized view k1_k2 as select k1,k2 from " + TEST_TABLE_NAME + " group by " + + "k1,k2;"; + String query = "select k1 from " + TEST_TABLE_NAME + " group by k1 order by max(v1);"; + dorisAssert.withMaterializedView(createK1K2MV).query(query).explainWithout("k1_k2"); + dorisAssert.dropTable(TEST_TABLE_NAME); + } + + @Test + public void testWindowsFunctionInQuery() throws Exception { + String duplicateTable = "CREATE TABLE " + TEST_TABLE_NAME + " ( k1 int(11) NOT NULL , k2 int(11) NOT NULL ," + + "v1 varchar(4096) NOT NULL, v2 float NOT NULL , v3 decimal(20, 7) NOT NULL ) ENGINE=OLAP " + + "DUPLICATE KEY( k1 , k2 ) DISTRIBUTED BY HASH( k1 , k2 ) BUCKETS 3 " + + "PROPERTIES ('replication_num' = '1'); "; + dorisAssert.withTable(duplicateTable); + String createK1K2MV = "create materialized view k1_k2 as select k1,k2 from " + TEST_TABLE_NAME + " group by " + + "k1,k2;"; + String query = "select k1 , sum(k2) over (partition by v1 ) from " + TEST_TABLE_NAME + ";"; + dorisAssert.withMaterializedView(createK1K2MV).query(query).explainWithout("k1_k2"); + dorisAssert.dropTable(TEST_TABLE_NAME); + } } diff --git a/fe/src/test/java/org/apache/doris/planner/MaterializedViewSelectorTest.java b/fe/src/test/java/org/apache/doris/planner/MaterializedViewSelectorTest.java index d5648b9d2d..6dfb9b8561 100644 --- a/fe/src/test/java/org/apache/doris/planner/MaterializedViewSelectorTest.java +++ b/fe/src/test/java/org/apache/doris/planner/MaterializedViewSelectorTest.java @@ -108,18 +108,8 @@ public class MaterializedViewSelectorTest { tableBColumn1.getTableName().getTbl(); result = "tableB"; - selectStmt.getResultExprs(); - result = Lists.newArrayList(tableAColumn1, tableAColumn2Sum, tableBColumn1Max); - tableAColumn2Desc.isMaterialized(); - result = true; - tableAColumn2Desc.getColumn().getName(); - result = "c2"; tableAColumn2Desc.getParent(); result = tableADesc; - tableBColumn1Desc.isMaterialized(); - result = true; - tableBColumn1Desc.getColumn().getName(); - result = "c1"; tableBColumn1Desc.getParent(); result = tableBDesc; tableBDesc.getTable(); @@ -154,16 +144,6 @@ public class MaterializedViewSelectorTest { MaterializedViewSelector.AggregatedColumn aggregatedColumn2 = tableBAgggregatedColumns.iterator().next(); Assert.assertEquals("c1", Deencapsulation.getField(aggregatedColumn2, "columnName")); Assert.assertTrue("MAX".equalsIgnoreCase(Deencapsulation.getField(aggregatedColumn2, "aggFunctionName"))); - Map> columnNamesInQueryOutput = - Deencapsulation.getField(materializedViewSelector, "columnNamesInQueryOutput"); - Assert.assertEquals(2, columnNamesInQueryOutput.size()); - Set tableAColumnNamesInQueryOutput = columnNamesInQueryOutput.get("tableA"); - Assert.assertEquals(2, tableAColumnNamesInQueryOutput.size()); - Assert.assertTrue(tableAColumnNamesInQueryOutput.contains("c1")); - Assert.assertTrue(tableAColumnNamesInQueryOutput.contains("c2")); - Set tableBColumnNamesInQueryOutput = columnNamesInQueryOutput.get("tableB"); - Assert.assertEquals(1, tableBColumnNamesInQueryOutput.size()); - Assert.assertTrue(tableBColumnNamesInQueryOutput.contains("c1")); } @Test @@ -195,8 +175,6 @@ public class MaterializedViewSelectorTest { { selectStmt.getAggInfo(); result = null; - selectStmt.getResultExprs(); - result = Lists.newArrayList(); indexMeta1.getSchema(); result = index1Columns; indexMeta2.getSchema(); @@ -243,8 +221,6 @@ public class MaterializedViewSelectorTest { { selectStmt.getAggInfo(); result = null; - selectStmt.getResultExprs(); - result = Lists.newArrayList(); indexMeta1.getSchema(); result = index1Columns; indexMeta1.getKeysType(); @@ -290,8 +266,6 @@ public class MaterializedViewSelectorTest { { selectStmt.getAggInfo(); result = null; - selectStmt.getResultExprs(); - result = Lists.newArrayList(); indexMeta1.getSchema(); result = index1Columns; indexMeta2.getSchema(); @@ -340,8 +314,6 @@ public class MaterializedViewSelectorTest { { selectStmt.getAggInfo(); result = null; - selectStmt.getResultExprs(); - result = Lists.newArrayList(); indexMeta1.getSchema(); result = index1Columns; indexMeta2.getSchema(); @@ -389,8 +361,6 @@ public class MaterializedViewSelectorTest { { selectStmt.getAggInfo(); result = null; - selectStmt.getResultExprs(); - result = Lists.newArrayList(); table.getBaseIndexId(); result = -1L; table.getKeyColumnsByIndexId(-1L); @@ -443,8 +413,6 @@ public class MaterializedViewSelectorTest { { selectStmt.getAggInfo(); result = null; - selectStmt.getResultExprs(); - result = Lists.newArrayList(); } }; Set equivalenceColumns = Sets.newHashSet();