From 606d17ea3d206e9febdd50b79df2e6710a9aa16a Mon Sep 17 00:00:00 2001 From: emmymiao87 <522274284@qq.com> Date: Tue, 9 Jun 2020 17:14:54 +0800 Subject: [PATCH 01/13] Forbidden float column in short key When the user does not specify the key column, doris will automatically supplement the key column. However, doris does not support float or double as the key column, so when adding the key column, doris should avoid setting those column as the key column. The CreateMaterailizedView, AddRollup and CreateDuplicateTable need to forbidden float column in short key. If the float column is directly encountered during the supplement process, the subsequent columns are all value columns. If the first column is float or double, Doris will throw the exception. Fixed #3811 Change-Id: Ib66d9355acefcd8f281906bcb7b4dd684319ff08 --- .../doris/alter/MaterializedViewHandler.java | 12 +- .../analysis/CreateMaterializedViewStmt.java | 13 +- .../doris/analysis/CreateTableStmt.java | 13 +- .../apache/doris/analysis/MVColumnItem.java | 11 ++ .../CreateMaterializedViewStmtTest.java | 159 ++++++++++++++++++ 5 files changed, 201 insertions(+), 7 deletions(-) diff --git a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java index 37136a00dd2808..a13966aab2cbfb 100644 --- a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java +++ b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java @@ -481,7 +481,7 @@ private List checkAndPrepareMaterializedView(CreateMaterializedViewStmt public List checkAndPrepareMaterializedView(AddRollupClause addRollupClause, OlapTable olapTable, long baseIndexId, boolean changeStorageFormat) - throws DdlException { + throws DdlException{ String rollupIndexName = addRollupClause.getRollupName(); List rollupColumnNames = addRollupClause.getColumnNames(); if (changeStorageFormat) { @@ -561,13 +561,19 @@ public List checkAndPrepareMaterializedView(AddRollupClause addRollupCla if (baseColumn == null) { throw new DdlException("Column[" + columnName + "] does not exist in base index"); } + // Supplement key of MV columns + if (baseColumn.getType().getPrimitiveType().isFloatingPointType() && i == 0) { + throw new DdlException("The first column could not be float or double, " + + "use decimal instead."); + } keyStorageLayoutBytes += baseColumn.getType().getStorageLayoutBytes(); Column rollupColumn = new Column(baseColumn); if(changeStorageFormat) { rollupColumn.setIsKey(baseColumn.isKey()); rollupColumn.setAggregationType(baseColumn.getAggregationType(), true); - } else if ((i + 1) <= FeConstants.shortkey_max_column_count - || keyStorageLayoutBytes < FeConstants.shortkey_maxsize_bytes) { + } else if (!rollupColumn.getType().getPrimitiveType() + .isFloatingPointType() && ((i + 1) <= FeConstants.shortkey_max_column_count || + keyStorageLayoutBytes < FeConstants.shortkey_maxsize_bytes)) { rollupColumn.setIsKey(true); rollupColumn.setAggregationType(null, false); } else { diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java index 56ec28d7693f5c..9d66bf96ccdb62 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java @@ -164,6 +164,7 @@ public void analyzeSelectClause() throws AnalysisException { ErrorReport.reportAnalysisException(ErrorCode.ERR_DUP_FIELDNAME, columnName); } MVColumnItem mvColumnItem = new MVColumnItem(columnName); + mvColumnItem.setType(slotRef.getType().getPrimitiveType()); mvColumnItemList.add(mvColumnItem); } else if (selectListItem.getExpr() instanceof FunctionCallExpr) { FunctionCallExpr functionCallExpr = (FunctionCallExpr) selectListItem.getExpr(); @@ -235,6 +236,9 @@ private void analyzeOrderByClause() throws AnalysisException { if (mvColumnItem.getAggregationType() != null) { break; } + if (mvColumnItem.getType().isFloatingPointType()) { + throw new AnalysisException("Float or double can not used as a key, use decimal instead."); + } mvColumnItem.setIsKey(true); } return; @@ -255,10 +259,15 @@ private void analyzeOrderByClause() throws AnalysisException { MVColumnItem mvColumnItem = mvColumnItemList.get(i); Expr resultColumn = selectStmt.getResultExprs().get(i); keyStorageLayoutBytes += resultColumn.getType().getStorageLayoutBytes(); - if ((i + 1) <= FeConstants.shortkey_max_column_count - || keyStorageLayoutBytes < FeConstants.shortkey_maxsize_bytes) { + if ((!mvColumnItem.getType().isFloatingPointType()) + && ((i + 1) <= FeConstants.shortkey_max_column_count + || keyStorageLayoutBytes < FeConstants.shortkey_maxsize_bytes)) { mvColumnItem.setIsKey(true); } else { + if (i == 0) { + throw new AnalysisException("The first column could not be float or double, " + + "use decimal instead."); + } mvColumnItem.setAggregationType(AggregateType.NONE, true); } } diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java index 9e7cd580ea68a8..ad6226f3d0da9e 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java @@ -284,11 +284,20 @@ public void analyze(Analyzer analyzer) throws UserException { } else { for (ColumnDef columnDef : columnDefs) { keyLength += columnDef.getType().getStorageLayoutBytes(); - if (keysColumnNames.size() < FeConstants.shortkey_max_column_count - || keyLength < FeConstants.shortkey_maxsize_bytes) { + if ((!columnDef.getType().getPrimitiveType().isFloatingPointType()) + && ((keysColumnNames.size() < FeConstants.shortkey_max_column_count) + || (keyLength < FeConstants.shortkey_maxsize_bytes))) { keysColumnNames.add(columnDef.getName()); + } else { + break; } } + // The OLAP table must has at least one short key and the float and double should not be short key. + // So the float and double could not be the first column in OLAP table. + if (keysColumnNames.isEmpty()) { + throw new AnalysisException( + "The first column could not be float or double, " + "use decimal instead."); + } keysDesc = new KeysDesc(KeysType.DUP_KEYS, keysColumnNames); } } diff --git a/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java b/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java index a78e216962002e..fc5523b7d11db0 100644 --- a/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java +++ b/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java @@ -18,6 +18,7 @@ package org.apache.doris.analysis; import org.apache.doris.catalog.AggregateType; +import org.apache.doris.catalog.PrimitiveType; /** * This is a result of semantic analysis for AddMaterializedViewClause. @@ -27,6 +28,8 @@ */ public class MVColumnItem { private String name; + // the origin type of slot ref + private PrimitiveType type; private boolean isKey; private AggregateType aggregationType; private boolean isAggregationTypeImplicit; @@ -40,6 +43,14 @@ public String getName() { return name; } + public void setType(PrimitiveType type) { + this.type = type; + } + + public PrimitiveType getType() { + return type; + } + public void setIsKey(boolean key) { isKey = key; } diff --git a/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java b/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java index de6c5a30edfdb2..9f0c7f801c9a61 100644 --- a/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java +++ b/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java @@ -19,6 +19,7 @@ import org.apache.doris.catalog.AggregateType; import org.apache.doris.catalog.KeysType; +import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.common.Config; import org.apache.doris.common.UserException; import org.apache.doris.common.jmockit.Deencapsulation; @@ -588,12 +589,20 @@ public void testMVColumnsWithoutOrderbyWithoutAggregation(@Injectable SlotRef sl result = Lists.newArrayList(slotRef1, slotRef2, slotRef3, slotRef4); slotRef1.getType().getStorageLayoutBytes(); result = 35; + slotRef1.getType().getPrimitiveType(); + result = PrimitiveType.INT; slotRef2.getType().getStorageLayoutBytes(); result = 2; + slotRef2.getType().getPrimitiveType(); + result = PrimitiveType.INT; slotRef3.getType().getStorageLayoutBytes(); result = 3; + slotRef3.getType().getPrimitiveType(); + result = PrimitiveType.INT; slotRef4.getType().getStorageLayoutBytes(); result = 4; + slotRef4.getType().getPrimitiveType(); + result = PrimitiveType.INT; selectStmt.getAggInfo(); // return null, so that the mv can be a duplicate mv result = null; } @@ -631,6 +640,156 @@ public void testMVColumnsWithoutOrderbyWithoutAggregation(@Injectable SlotRef sl } } + @Test + public void testMVColumnsWithoutOrderbyWithoutAggregationWithFloat(@Injectable SlotRef slotRef1, + @Injectable SlotRef slotRef2, @Injectable SlotRef slotRef3, @Injectable SlotRef slotRef4, + @Injectable TableRef tableRef, @Injectable SelectStmt selectStmt) throws UserException { + SelectList selectList = new SelectList(); + SelectListItem selectListItem1 = new SelectListItem(slotRef1, null); + selectList.addItem(selectListItem1); + SelectListItem selectListItem2 = new SelectListItem(slotRef2, null); + selectList.addItem(selectListItem2); + SelectListItem selectListItem3 = new SelectListItem(slotRef3, null); + selectList.addItem(selectListItem3); + SelectListItem selectListItem4 = new SelectListItem(slotRef4, null); + selectList.addItem(selectListItem4); + + final String columnName1 = "k1"; + final String columnName2 = "k2"; + final String columnName3 = "v1"; + final String columnName4 = "v2"; + + new Expectations() { + { + analyzer.getClusterName(); + result = "default"; + selectStmt.getAggInfo(); + result = null; + selectStmt.getSelectList(); + result = selectList; + selectStmt.getTableRefs(); + result = Lists.newArrayList(tableRef); + selectStmt.getWhereClause(); + result = null; + selectStmt.getHavingPred(); + result = null; + selectStmt.getOrderByElements(); + result = null; + selectStmt.getLimit(); + result = -1; + selectStmt.analyze(analyzer); + slotRef1.getColumnName(); + result = columnName1; + slotRef2.getColumnName(); + result = columnName2; + slotRef3.getColumnName(); + result = columnName3; + slotRef4.getColumnName(); + result = columnName4; + selectStmt.getResultExprs(); + result = Lists.newArrayList(slotRef1, slotRef2, slotRef3, slotRef4); + slotRef1.getType().getStorageLayoutBytes(); + result = 35; + slotRef1.getType().getPrimitiveType(); + result = PrimitiveType.INT; + slotRef2.getType().getStorageLayoutBytes(); + result = 2; + slotRef2.getType().getPrimitiveType(); + result = PrimitiveType.INT; + slotRef3.getType().getStorageLayoutBytes(); + result = 3; + slotRef3.getType().getPrimitiveType(); + result = PrimitiveType.FLOAT; + slotRef4.getType().getStorageLayoutBytes(); + result = 4; + slotRef4.getType().getPrimitiveType(); + result = PrimitiveType.INT; + selectStmt.getAggInfo(); // return null, so that the mv can be a duplicate mv + result = null; + } + }; + + + CreateMaterializedViewStmt createMaterializedViewStmt = new CreateMaterializedViewStmt("test", selectStmt, null); + try { + createMaterializedViewStmt.analyze(analyzer); + Assert.assertEquals(KeysType.DUP_KEYS, createMaterializedViewStmt.getMVKeysType()); + List mvColumns = createMaterializedViewStmt.getMVColumnItemList(); + Assert.assertEquals(4, mvColumns.size()); + MVColumnItem mvColumn0 = mvColumns.get(0); + Assert.assertTrue(mvColumn0.isKey()); + Assert.assertFalse(mvColumn0.isAggregationTypeImplicit()); + Assert.assertEquals(columnName1, mvColumn0.getName()); + Assert.assertEquals(null, mvColumn0.getAggregationType()); + MVColumnItem mvColumn1 = mvColumns.get(1); + Assert.assertTrue(mvColumn1.isKey()); + Assert.assertFalse(mvColumn1.isAggregationTypeImplicit()); + Assert.assertEquals(columnName2, mvColumn1.getName()); + Assert.assertEquals(null, mvColumn1.getAggregationType()); + MVColumnItem mvColumn2 = mvColumns.get(2); + Assert.assertFalse(mvColumn2.isKey()); + Assert.assertTrue(mvColumn2.isAggregationTypeImplicit()); + Assert.assertEquals(columnName3, mvColumn2.getName()); + Assert.assertEquals(AggregateType.NONE, mvColumn2.getAggregationType()); + MVColumnItem mvColumn3 = mvColumns.get(3); + Assert.assertFalse(mvColumn3.isKey()); + Assert.assertTrue(mvColumn3.isAggregationTypeImplicit()); + Assert.assertEquals(columnName4, mvColumn3.getName()); + Assert.assertEquals(AggregateType.NONE, mvColumn3.getAggregationType()); + } catch (UserException e) { + Assert.fail(e.getMessage()); + } + } + + @Test + public void testMVColumnsWithFirstFloat(@Injectable SlotRef slotRef1, + @Injectable TableRef tableRef, @Injectable SelectStmt selectStmt) throws UserException { + SelectList selectList = new SelectList(); + SelectListItem selectListItem1 = new SelectListItem(slotRef1, null); + selectList.addItem(selectListItem1); + + final String columnName1 = "k1"; + + new Expectations() { + { + analyzer.getClusterName(); + result = "default"; + selectStmt.getAggInfo(); + result = null; + selectStmt.getSelectList(); + result = selectList; + selectStmt.getTableRefs(); + result = Lists.newArrayList(tableRef); + selectStmt.getWhereClause(); + result = null; + selectStmt.getHavingPred(); + result = null; + selectStmt.getOrderByElements(); + result = null; + selectStmt.analyze(analyzer); + slotRef1.getColumnName(); + result = columnName1; + selectStmt.getResultExprs(); + result = Lists.newArrayList(slotRef1); + slotRef1.getType().getStorageLayoutBytes(); + result = 35; + slotRef1.getType().getPrimitiveType(); + result = PrimitiveType.FLOAT; + selectStmt.getAggInfo(); // return null, so that the mv can be a duplicate mv + result = null; + } + }; + + + CreateMaterializedViewStmt createMaterializedViewStmt = new CreateMaterializedViewStmt("test", selectStmt, null); + try { + createMaterializedViewStmt.analyze(analyzer); + Assert.fail("The first column could not be float or double, use decimal instead"); + } catch (UserException e) { + System.out.print(e.getMessage()); + } + } + @Test public void testMVColumns(@Injectable SlotRef slotRef1, @Injectable SlotRef slotRef2, From 3a32e578af3b7d1da216a30029e6411cc153b1e5 Mon Sep 17 00:00:00 2001 From: EmmyMiao87 <522274284@qq.com> Date: Wed, 10 Jun 2020 10:26:52 +0800 Subject: [PATCH 02/13] Update fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java Co-authored-by: Mingyu Chen --- .../org/apache/doris/analysis/CreateMaterializedViewStmt.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java index 9d66bf96ccdb62..4e75a080d51fbb 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java @@ -237,7 +237,7 @@ private void analyzeOrderByClause() throws AnalysisException { break; } if (mvColumnItem.getType().isFloatingPointType()) { - throw new AnalysisException("Float or double can not used as a key, use decimal instead."); + throw new AnalysisException("Float or double can not used as a sort key, use decimal instead."); } mvColumnItem.setIsKey(true); } From a85fd5db48570b80393a00c0fc9e108158021ef0 Mon Sep 17 00:00:00 2001 From: emmymiao87 <522274284@qq.com> Date: Wed, 10 Jun 2020 15:28:37 +0800 Subject: [PATCH 03/13] Fix bug Change-Id: Id5236ff7229fc1c1d35df579dd3c62e699392ba5 --- .../doris/alter/MaterializedViewHandler.java | 46 +++--- .../analysis/CreateMaterializedViewStmt.java | 135 +++++++++--------- .../CreateMaterializedViewStmtTest.java | 10 +- .../org/apache/doris/utframe/DorisAssert.java | 8 +- 4 files changed, 107 insertions(+), 92 deletions(-) diff --git a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java index a13966aab2cbfb..b9296b78afb4a5 100644 --- a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java +++ b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java @@ -554,34 +554,42 @@ public List checkAndPrepareMaterializedView(AddRollupClause addRollupCla } else if (KeysType.DUP_KEYS == keysType) { // supplement the duplicate key if (addRollupClause.getDupKeys() == null || addRollupClause.getDupKeys().isEmpty()) { - int keyStorageLayoutBytes = 0; + // check the column meta for (int i = 0; i < rollupColumnNames.size(); i++) { String columnName = rollupColumnNames.get(i); Column baseColumn = baseColumnNameToColumn.get(columnName); if (baseColumn == null) { throw new DdlException("Column[" + columnName + "] does not exist in base index"); } - // Supplement key of MV columns - if (baseColumn.getType().getPrimitiveType().isFloatingPointType() && i == 0) { - throw new DdlException("The first column could not be float or double, " - + "use decimal instead."); - } - keyStorageLayoutBytes += baseColumn.getType().getStorageLayoutBytes(); Column rollupColumn = new Column(baseColumn); - if(changeStorageFormat) { - rollupColumn.setIsKey(baseColumn.isKey()); - rollupColumn.setAggregationType(baseColumn.getAggregationType(), true); - } else if (!rollupColumn.getType().getPrimitiveType() - .isFloatingPointType() && ((i + 1) <= FeConstants.shortkey_max_column_count || - keyStorageLayoutBytes < FeConstants.shortkey_maxsize_bytes)) { - rollupColumn.setIsKey(true); - rollupColumn.setAggregationType(null, false); - } else { - rollupColumn.setIsKey(false); - rollupColumn.setAggregationType(AggregateType.NONE, true); - } rollupSchema.add(rollupColumn); } + if (changeStorageFormat) { + return rollupSchema; + } + // Supplement short key of MV columns + int theBeginIndexOfValue = 0; + int keyStorageLayoutBytes = 0; + for (; theBeginIndexOfValue < rollupSchema.size(); theBeginIndexOfValue++) { + Column rollupColumn = rollupSchema.get(theBeginIndexOfValue); + keyStorageLayoutBytes += rollupColumn.getType().getStorageLayoutBytes(); + if (rollupColumn.getType().getPrimitiveType().isFloatingPointType() + || ((theBeginIndexOfValue + 1) > FeConstants.shortkey_max_column_count) + && (keyStorageLayoutBytes > FeConstants.shortkey_maxsize_bytes)) { + break; + } + rollupColumn.setIsKey(true); + rollupColumn.setAggregationType(null, false); + } + if (theBeginIndexOfValue == 0) { + throw new DdlException("The first column could not be float or double, use decimal instead."); + } + // Supplement value of MV columns + for (; theBeginIndexOfValue < rollupSchema.size(); theBeginIndexOfValue++) { + Column rollupColumn = rollupSchema.get(theBeginIndexOfValue); + rollupColumn.setIsKey(false); + rollupColumn.setAggregationType(AggregateType.NONE, true); + } } else { /* * eg. diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java index 4e75a080d51fbb..77b25a8beab7fd 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java @@ -69,8 +69,7 @@ public class CreateMaterializedViewStmt extends DdlStmt { private String dbName; private KeysType mvKeysType = KeysType.DUP_KEYS; - public CreateMaterializedViewStmt(String mvName, SelectStmt selectStmt, - Map properties) { + public CreateMaterializedViewStmt(String mvName, SelectStmt selectStmt, Map properties) { this.mvName = mvName; this.selectStmt = selectStmt; this.properties = properties; @@ -115,17 +114,14 @@ public void analyze(Analyzer analyzer) throws UserException { analyzeSelectClause(); analyzeFromClause(); if (selectStmt.getWhereClause() != null) { - throw new AnalysisException("The where clause is not supported in add materialized view clause, expr:" - + selectStmt.getWhereClause().toSql()); + throw new AnalysisException("The where clause is not supported in add materialized view clause, expr:" + selectStmt.getWhereClause().toSql()); } if (selectStmt.getHavingPred() != null) { - throw new AnalysisException("The having clause is not supported in add materialized view clause, expr:" - + selectStmt.getHavingPred().toSql()); + throw new AnalysisException("The having clause is not supported in add materialized view clause, expr:" + selectStmt.getHavingPred().toSql()); } analyzeOrderByClause(); if (selectStmt.getLimit() != -1) { - throw new AnalysisException("The limit clause is not supported in add materialized view clause, expr:" - + " limit " + selectStmt.getLimit()); + throw new AnalysisException("The limit clause is not supported in add materialized view clause, expr:" + " limit " + selectStmt.getLimit()); } } @@ -150,8 +146,7 @@ public void analyzeSelectClause() throws AnalysisException { SelectListItem selectListItem = selectList.getItems().get(i); Expr selectListItemExpr = selectListItem.getExpr(); if (!(selectListItemExpr instanceof SlotRef) && !(selectListItemExpr instanceof FunctionCallExpr)) { - throw new AnalysisException("The materialized view only support the single column or function expr. " - + "Error column: " + selectListItemExpr.toSql()); + throw new AnalysisException("The materialized view only support the single column or function expr. " + "Error column: " + selectListItemExpr.toSql()); } if (selectListItem.getExpr() instanceof SlotRef) { if (meetAggregate) { @@ -171,11 +166,8 @@ public void analyzeSelectClause() throws AnalysisException { 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")) { - throw new AnalysisException("The materialized view only support the sum, min and max aggregate " - + "function. Error function: " + functionCallExpr.toSqlImpl()); + if (!functionName.equalsIgnoreCase("sum") && !functionName.equalsIgnoreCase("min") && !functionName.equalsIgnoreCase("max")) { + 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); @@ -183,13 +175,10 @@ public void analyzeSelectClause() throws AnalysisException { SlotRef slotRef; if (functionChild0 instanceof SlotRef) { slotRef = (SlotRef) functionChild0; - } - else if (functionChild0 instanceof CastExpr - && (functionChild0.getChild(0) instanceof SlotRef)) { + } else if (functionChild0 instanceof CastExpr && (functionChild0.getChild(0) instanceof SlotRef)) { slotRef = (SlotRef) functionChild0.getChild(0); } else { - throw new AnalysisException("The children of aggregate function only support one original column. " - + "Error function: " + functionCallExpr.toSqlImpl()); + throw new AnalysisException("The children of aggregate function only support one original column. " + "Error function: " + functionCallExpr.toSqlImpl()); } meetAggregate = true; // check duplicate column @@ -227,57 +216,14 @@ private void analyzeFromClause() throws AnalysisException { private void analyzeOrderByClause() throws AnalysisException { if (selectStmt.getOrderByElements() == null) { - /** - * The keys type of Materialized view is aggregation. - * All of group by columns are keys of materialized view. - */ - if (mvKeysType == KeysType.AGG_KEYS) { - for (MVColumnItem mvColumnItem : mvColumnItemList) { - if (mvColumnItem.getAggregationType() != null) { - break; - } - if (mvColumnItem.getType().isFloatingPointType()) { - throw new AnalysisException("Float or double can not used as a sort key, use decimal instead."); - } - mvColumnItem.setIsKey(true); - } - return; - } - - /** - * There is no aggregation function in materialized view. - * Supplement key of MV columns - * For example: select k1, k2 ... kn from t1 - * The default key columns are first 36 bytes of the columns in define order. - * If the number of columns in the first 36 is less than 3, the first 3 columns will be used. - * column: k1, k2, k3... km. The key is true. - * Supplement non-key of MV columns - * column: km... kn. The key is false, aggregation type is none, isAggregationTypeImplicit is true. - */ - int keyStorageLayoutBytes = 0; - for (int i = 0; i < selectStmt.getResultExprs().size(); i++) { - MVColumnItem mvColumnItem = mvColumnItemList.get(i); - Expr resultColumn = selectStmt.getResultExprs().get(i); - keyStorageLayoutBytes += resultColumn.getType().getStorageLayoutBytes(); - if ((!mvColumnItem.getType().isFloatingPointType()) - && ((i + 1) <= FeConstants.shortkey_max_column_count - || keyStorageLayoutBytes < FeConstants.shortkey_maxsize_bytes)) { - mvColumnItem.setIsKey(true); - } else { - if (i == 0) { - throw new AnalysisException("The first column could not be float or double, " - + "use decimal instead."); - } - mvColumnItem.setAggregationType(AggregateType.NONE, true); - } - } + supplyShortKey(); return; } List orderByElements = selectStmt.getOrderByElements(); if (orderByElements.size() > mvColumnItemList.size()) { - throw new AnalysisException("The number of columns in order clause must be less then " - + "the number of columns in select clause"); + throw new AnalysisException("The number of columns in order clause must be less then " + "the number of " + + "columns in select clause"); } if (beginIndexOfAggregation != -1 && (orderByElements.size() != (beginIndexOfAggregation))) { throw new AnalysisException("The key of columns in mv must be all of group by columns"); @@ -286,13 +232,13 @@ private void analyzeOrderByClause() throws AnalysisException { Expr orderByElement = orderByElements.get(i).getExpr(); if (!(orderByElement instanceof SlotRef)) { throw new AnalysisException("The column in order clause must be original column without calculation. " - + "Error column: " + orderByElement.toSql()); + + "Error column: " + orderByElement.toSql()); } MVColumnItem mvColumnItem = mvColumnItemList.get(i); SlotRef slotRef = (SlotRef) orderByElement; if (!mvColumnItem.getName().equalsIgnoreCase(slotRef.getColumnName())) { throw new AnalysisException("The order of columns in order by clause must be same as " - + "the order of columns in select list"); + + "the order of columns in select list"); } Preconditions.checkState(mvColumnItem.getAggregationType() == null); mvColumnItem.setIsKey(true); @@ -310,6 +256,59 @@ private void analyzeOrderByClause() throws AnalysisException { } } + private void supplyShortKey() throws AnalysisException { + /** + * The keys type of Materialized view is aggregation. + * All of group by columns are keys of materialized view. + */ + if (mvKeysType == KeysType.AGG_KEYS) { + for (MVColumnItem mvColumnItem : mvColumnItemList) { + if (mvColumnItem.getAggregationType() != null) { + break; + } + if (mvColumnItem.getType().isFloatingPointType()) { + throw new AnalysisException("Float or double can not used as a key, use decimal instead."); + } + mvColumnItem.setIsKey(true); + } + return; + } + + /** + * There is no aggregation function in materialized view. + * Supplement key of MV columns + * For example: select k1, k2 ... kn from t1 + * The default key columns are first 36 bytes of the columns in define order. + * If the number of columns in the first 36 is less than 3, the first 3 columns will be used. + * column: k1, k2, k3... km. The key is true. + * Supplement non-key of MV columns + * column: km... kn. The key is false, aggregation type is none, isAggregationTypeImplicit is true. + */ + int keyStorageLayoutBytes = 0; + int theBeginIndexOfValue = 0; + // supply short key + for (; theBeginIndexOfValue < selectStmt.getResultExprs().size(); theBeginIndexOfValue++) { + MVColumnItem mvColumnItem = mvColumnItemList.get(theBeginIndexOfValue); + Expr resultColumn = selectStmt.getResultExprs().get(theBeginIndexOfValue); + keyStorageLayoutBytes += resultColumn.getType().getStorageLayoutBytes(); + if (mvColumnItem.getType().isFloatingPointType() + ||((theBeginIndexOfValue + 1) > FeConstants.shortkey_max_column_count) + && (keyStorageLayoutBytes > FeConstants.shortkey_maxsize_bytes)) { + break; + } + mvColumnItem.setIsKey(true); + } + if (theBeginIndexOfValue == 0) { + throw new AnalysisException("The first column could not be float or double, use decimal instead."); + } + // supply value + for (; theBeginIndexOfValue < selectStmt.getResultExprs().size(); theBeginIndexOfValue++) { + MVColumnItem mvColumnItem = mvColumnItemList.get(theBeginIndexOfValue); + mvColumnItem.setAggregationType(AggregateType.NONE, true); + } + + } + @Override public String toSql() { return null; diff --git a/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java b/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java index 9f0c7f801c9a61..fdce6b5d7c6c8e 100644 --- a/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java +++ b/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java @@ -640,6 +640,9 @@ public void testMVColumnsWithoutOrderbyWithoutAggregation(@Injectable SlotRef sl } } + /* + ISSUE: #3811 + */ @Test public void testMVColumnsWithoutOrderbyWithoutAggregationWithFloat(@Injectable SlotRef slotRef1, @Injectable SlotRef slotRef2, @Injectable SlotRef slotRef3, @Injectable SlotRef slotRef4, @@ -689,7 +692,7 @@ public void testMVColumnsWithoutOrderbyWithoutAggregationWithFloat(@Injectable S selectStmt.getResultExprs(); result = Lists.newArrayList(slotRef1, slotRef2, slotRef3, slotRef4); slotRef1.getType().getStorageLayoutBytes(); - result = 35; + result = 1; slotRef1.getType().getPrimitiveType(); result = PrimitiveType.INT; slotRef2.getType().getStorageLayoutBytes(); @@ -700,8 +703,6 @@ public void testMVColumnsWithoutOrderbyWithoutAggregationWithFloat(@Injectable S result = 3; slotRef3.getType().getPrimitiveType(); result = PrimitiveType.FLOAT; - slotRef4.getType().getStorageLayoutBytes(); - result = 4; slotRef4.getType().getPrimitiveType(); result = PrimitiveType.INT; selectStmt.getAggInfo(); // return null, so that the mv can be a duplicate mv @@ -741,6 +742,9 @@ public void testMVColumnsWithoutOrderbyWithoutAggregationWithFloat(@Injectable S } } + /* + ISSUE: #3811 + */ @Test public void testMVColumnsWithFirstFloat(@Injectable SlotRef slotRef1, @Injectable TableRef tableRef, @Injectable SelectStmt selectStmt) throws UserException { diff --git a/fe/src/test/java/org/apache/doris/utframe/DorisAssert.java b/fe/src/test/java/org/apache/doris/utframe/DorisAssert.java index b808fecec564ea..8ba14dab4dda0f 100644 --- a/fe/src/test/java/org/apache/doris/utframe/DorisAssert.java +++ b/fe/src/test/java/org/apache/doris/utframe/DorisAssert.java @@ -141,11 +141,15 @@ public void explainWithout(String s) throws Exception { } public String explainQuery() throws Exception { - StmtExecutor stmtExecutor = new StmtExecutor(connectContext, "explain " + sql); + return internalExecute("explain " + sql); + } + + private String internalExecute(String sql) throws Exception { + StmtExecutor stmtExecutor = new StmtExecutor(connectContext, sql); stmtExecutor.execute(); QueryState queryState = connectContext.getState(); if (queryState.getStateType() == QueryState.MysqlStateType.ERR) { - switch (queryState.getErrType()){ + switch (queryState.getErrType()) { case ANALYSIS_ERR: throw new AnalysisException(queryState.getErrorMessage()); case OTHER_ERR: From 8b8b5c5b7d1487d657679d58c3203e670b48bf3c Mon Sep 17 00:00:00 2001 From: emmymiao87 <522274284@qq.com> Date: Wed, 10 Jun 2020 15:33:25 +0800 Subject: [PATCH 04/13] Change the error msg Change-Id: Ie1320d9af56083633c7634c1c3df485f3994da31 --- fe/src/main/java/org/apache/doris/analysis/ColumnDef.java | 2 +- .../main/java/org/apache/doris/analysis/CreateTableStmt.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java b/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java index d51e6edfeaf0bd..e1832a55c64631 100644 --- a/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java +++ b/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java @@ -158,7 +158,7 @@ public void analyze(boolean isOlap) throws AnalysisException { if (type.getPrimitiveType() == PrimitiveType.FLOAT || type.getPrimitiveType() == PrimitiveType.DOUBLE) { if (isOlap && isKey) { - throw new AnalysisException("Float or double can not used as a key, use decimal instead."); + throw new AnalysisException("Float or double can not used as a short key, use decimal instead."); } } diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java index ad6226f3d0da9e..ec603c0d9eb248 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java @@ -295,8 +295,8 @@ public void analyze(Analyzer analyzer) throws UserException { // The OLAP table must has at least one short key and the float and double should not be short key. // So the float and double could not be the first column in OLAP table. if (keysColumnNames.isEmpty()) { - throw new AnalysisException( - "The first column could not be float or double, " + "use decimal instead."); + throw new AnalysisException("The first column could not be float or double," + + " use decimal instead."); } keysDesc = new KeysDesc(KeysType.DUP_KEYS, keysColumnNames); } From 6ec2aa18b5e3aba471456883b03ac8c6fb8698ba Mon Sep 17 00:00:00 2001 From: emmymiao87 <522274284@qq.com> Date: Wed, 10 Jun 2020 15:35:38 +0800 Subject: [PATCH 05/13] Change code style Change-Id: Ib4c4791847d522fa312efff4016787c9caeab785 --- .../analysis/CreateMaterializedViewStmt.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java index 77b25a8beab7fd..7232d4bb221850 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java @@ -114,14 +114,17 @@ public void analyze(Analyzer analyzer) throws UserException { analyzeSelectClause(); analyzeFromClause(); if (selectStmt.getWhereClause() != null) { - throw new AnalysisException("The where clause is not supported in add materialized view clause, expr:" + selectStmt.getWhereClause().toSql()); + throw new AnalysisException("The where clause is not supported in add materialized view clause, expr:" + + selectStmt.getWhereClause().toSql()); } if (selectStmt.getHavingPred() != null) { - throw new AnalysisException("The having clause is not supported in add materialized view clause, expr:" + selectStmt.getHavingPred().toSql()); + throw new AnalysisException("The having clause is not supported in add materialized view clause, expr:" + + selectStmt.getHavingPred().toSql()); } analyzeOrderByClause(); if (selectStmt.getLimit() != -1) { - throw new AnalysisException("The limit clause is not supported in add materialized view clause, expr:" + " limit " + selectStmt.getLimit()); + throw new AnalysisException("The limit clause is not supported in add materialized view clause, expr:" + + " limit " + selectStmt.getLimit()); } } @@ -146,7 +149,8 @@ public void analyzeSelectClause() throws AnalysisException { SelectListItem selectListItem = selectList.getItems().get(i); Expr selectListItemExpr = selectListItem.getExpr(); if (!(selectListItemExpr instanceof SlotRef) && !(selectListItemExpr instanceof FunctionCallExpr)) { - throw new AnalysisException("The materialized view only support the single column or function expr. " + "Error column: " + selectListItemExpr.toSql()); + throw new AnalysisException("The materialized view only support the single column or function expr. " + + "Error column: " + selectListItemExpr.toSql()); } if (selectListItem.getExpr() instanceof SlotRef) { if (meetAggregate) { @@ -166,8 +170,11 @@ public void analyzeSelectClause() throws AnalysisException { 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")) { - throw new AnalysisException("The materialized view only support the sum, min and max aggregate " + "function. Error function: " + functionCallExpr.toSqlImpl()); + if (!functionName.equalsIgnoreCase("sum") + && !functionName.equalsIgnoreCase("min") + && !functionName.equalsIgnoreCase("max")) { + 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); @@ -178,7 +185,8 @@ public void analyzeSelectClause() throws AnalysisException { } else if (functionChild0 instanceof CastExpr && (functionChild0.getChild(0) instanceof SlotRef)) { slotRef = (SlotRef) functionChild0.getChild(0); } else { - throw new AnalysisException("The children of aggregate function only support one original column. " + "Error function: " + functionCallExpr.toSqlImpl()); + throw new AnalysisException("The children of aggregate function only support one original column. " + + "Error function: " + functionCallExpr.toSqlImpl()); } meetAggregate = true; // check duplicate column From b935490808aea3ca97e8a1d221993c8c2690a506 Mon Sep 17 00:00:00 2001 From: emmymiao87 <522274284@qq.com> Date: Wed, 10 Jun 2020 20:04:07 +0800 Subject: [PATCH 06/13] Change the strategy of order by column(key column) For duplicate table without order by columns, the order by columns are same as short key columns. If the order by columns have been designated, the count of short key columns must be <= the count of order by columns. The short key columns must be less then 3 columns and less then 36 bytes. Also the float and double could not be the short key column. At the same time, doirs must be at least one short key column. So the type of first column could not be float or double. If the varchar is the short key column, it can only be the least one short key column. Change-Id: I46ec2c78e87d73c3f3aeeb815b9acc6dcfe280e0 --- .../doris/alter/MaterializedViewHandler.java | 29 ++- .../org/apache/doris/analysis/ColumnDef.java | 2 +- .../analysis/CreateMaterializedViewStmt.java | 90 +++++---- .../doris/analysis/CreateTableStmt.java | 18 +- .../apache/doris/analysis/MVColumnItem.java | 11 +- .../org/apache/doris/catalog/Catalog.java | 33 ++-- .../java/org/apache/doris/catalog/Type.java | 8 + .../CreateMaterializedViewStmtTest.java | 176 ++++++++++++++++-- 8 files changed, 278 insertions(+), 89 deletions(-) diff --git a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java index b9296b78afb4a5..f4fcc06334b4be 100644 --- a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java +++ b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java @@ -36,6 +36,7 @@ import org.apache.doris.catalog.OlapTable; import org.apache.doris.catalog.OlapTable.OlapTableState; import org.apache.doris.catalog.Partition; +import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.catalog.Replica; import org.apache.doris.catalog.Table; import org.apache.doris.catalog.Tablet; @@ -567,19 +568,29 @@ public List checkAndPrepareMaterializedView(AddRollupClause addRollupCla if (changeStorageFormat) { return rollupSchema; } - // Supplement short key of MV columns + // Supplement key of MV columns int theBeginIndexOfValue = 0; - int keyStorageLayoutBytes = 0; + int keySizeByte = 0; for (; theBeginIndexOfValue < rollupSchema.size(); theBeginIndexOfValue++) { - Column rollupColumn = rollupSchema.get(theBeginIndexOfValue); - keyStorageLayoutBytes += rollupColumn.getType().getStorageLayoutBytes(); - if (rollupColumn.getType().getPrimitiveType().isFloatingPointType() - || ((theBeginIndexOfValue + 1) > FeConstants.shortkey_max_column_count) - && (keyStorageLayoutBytes > FeConstants.shortkey_maxsize_bytes)) { + Column column = rollupSchema.get(theBeginIndexOfValue); + keySizeByte += column.getType().getIndexSize(); + if (theBeginIndexOfValue + 1 > FeConstants.shortkey_max_column_count + || keySizeByte > FeConstants.shortkey_maxsize_bytes) { + if (theBeginIndexOfValue == 0 && column.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { + column.setIsKey(true); + theBeginIndexOfValue++; + } + break; + } + if (column.getType().isFloatingPointType()) { + break; + } + if (column.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { + column.setIsKey(true); + theBeginIndexOfValue++; break; } - rollupColumn.setIsKey(true); - rollupColumn.setAggregationType(null, false); + column.setIsKey(true); } if (theBeginIndexOfValue == 0) { throw new DdlException("The first column could not be float or double, use decimal instead."); diff --git a/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java b/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java index e1832a55c64631..d51e6edfeaf0bd 100644 --- a/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java +++ b/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java @@ -158,7 +158,7 @@ public void analyze(boolean isOlap) throws AnalysisException { if (type.getPrimitiveType() == PrimitiveType.FLOAT || type.getPrimitiveType() == PrimitiveType.DOUBLE) { if (isOlap && isKey) { - throw new AnalysisException("Float or double can not used as a short key, use decimal instead."); + throw new AnalysisException("Float or double can not used as a key, use decimal instead."); } } diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java index 7232d4bb221850..cb65fb8352f0f0 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java @@ -19,6 +19,7 @@ import org.apache.doris.catalog.AggregateType; import org.apache.doris.catalog.KeysType; +import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.ErrorCode; @@ -68,6 +69,7 @@ public class CreateMaterializedViewStmt extends DdlStmt { private String baseIndexName; private String dbName; private KeysType mvKeysType = KeysType.DUP_KEYS; + private int shortKeyColumnCount; public CreateMaterializedViewStmt(String mvName, SelectStmt selectStmt, Map properties) { this.mvName = mvName; @@ -163,7 +165,7 @@ public void analyzeSelectClause() throws AnalysisException { ErrorReport.reportAnalysisException(ErrorCode.ERR_DUP_FIELDNAME, columnName); } MVColumnItem mvColumnItem = new MVColumnItem(columnName); - mvColumnItem.setType(slotRef.getType().getPrimitiveType()); + mvColumnItem.setType(slotRef.getType()); mvColumnItemList.add(mvColumnItem); } else if (selectListItem.getExpr() instanceof FunctionCallExpr) { FunctionCallExpr functionCallExpr = (FunctionCallExpr) selectListItem.getExpr(); @@ -224,7 +226,7 @@ private void analyzeFromClause() throws AnalysisException { private void analyzeOrderByClause() throws AnalysisException { if (selectStmt.getOrderByElements() == null) { - supplyShortKey(); + supplyOrderColumn(); return; } @@ -264,7 +266,10 @@ private void analyzeOrderByClause() throws AnalysisException { } } - private void supplyShortKey() throws AnalysisException { + /* + This function is used to supply order by columns and calculate short key count + */ + private void supplyOrderColumn() throws AnalysisException { /** * The keys type of Materialized view is aggregation. * All of group by columns are keys of materialized view. @@ -274,45 +279,52 @@ private void supplyShortKey() throws AnalysisException { if (mvColumnItem.getAggregationType() != null) { break; } - if (mvColumnItem.getType().isFloatingPointType()) { - throw new AnalysisException("Float or double can not used as a key, use decimal instead."); - } mvColumnItem.setIsKey(true); } - return; - } - - /** - * There is no aggregation function in materialized view. - * Supplement key of MV columns - * For example: select k1, k2 ... kn from t1 - * The default key columns are first 36 bytes of the columns in define order. - * If the number of columns in the first 36 is less than 3, the first 3 columns will be used. - * column: k1, k2, k3... km. The key is true. - * Supplement non-key of MV columns - * column: km... kn. The key is false, aggregation type is none, isAggregationTypeImplicit is true. - */ - int keyStorageLayoutBytes = 0; - int theBeginIndexOfValue = 0; - // supply short key - for (; theBeginIndexOfValue < selectStmt.getResultExprs().size(); theBeginIndexOfValue++) { - MVColumnItem mvColumnItem = mvColumnItemList.get(theBeginIndexOfValue); - Expr resultColumn = selectStmt.getResultExprs().get(theBeginIndexOfValue); - keyStorageLayoutBytes += resultColumn.getType().getStorageLayoutBytes(); - if (mvColumnItem.getType().isFloatingPointType() - ||((theBeginIndexOfValue + 1) > FeConstants.shortkey_max_column_count) - && (keyStorageLayoutBytes > FeConstants.shortkey_maxsize_bytes)) { - break; + } else if (mvKeysType == KeysType.DUP_KEYS) { + /** + * There is no aggregation function in materialized view. + * Supplement key of MV columns + * The key is same as the short key in duplicate table + * For example: select k1, k2 ... kn from t1 + * The default key columns are first 36 bytes of the columns in define order. + * If the number of columns in the first 36 is more than 3, the first 3 columns will be used. + * column: k1, k2, k3. The key is true. + * Supplement non-key of MV columns + * column: k4... kn. The key is false, aggregation type is none, isAggregationTypeImplicit is true. + */ + int theBeginIndexOfValue = 0; + // supply key + int keySizeByte = 0; + for (; theBeginIndexOfValue < mvColumnItemList.size(); theBeginIndexOfValue++) { + MVColumnItem column = mvColumnItemList.get(theBeginIndexOfValue); + keySizeByte += column.getType().getIndexSize(); + if (theBeginIndexOfValue + 1 > FeConstants.shortkey_max_column_count + || keySizeByte > FeConstants.shortkey_maxsize_bytes) { + if (theBeginIndexOfValue == 0 && column.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { + column.setIsKey(true); + theBeginIndexOfValue++; + } + break; + } + if (column.getType().isFloatingPointType()) { + break; + } + if (column.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { + column.setIsKey(true); + theBeginIndexOfValue++; + break; + } + column.setIsKey(true); + } + if (theBeginIndexOfValue == 0) { + throw new AnalysisException("The first column could not be float or double type, use decimal instead"); + } + // supply value + for (; theBeginIndexOfValue < mvColumnItemList.size(); theBeginIndexOfValue++) { + MVColumnItem mvColumnItem = mvColumnItemList.get(theBeginIndexOfValue); + mvColumnItem.setAggregationType(AggregateType.NONE, true); } - mvColumnItem.setIsKey(true); - } - if (theBeginIndexOfValue == 0) { - throw new AnalysisException("The first column could not be float or double, use decimal instead."); - } - // supply value - for (; theBeginIndexOfValue < selectStmt.getResultExprs().size(); theBeginIndexOfValue++) { - MVColumnItem mvColumnItem = mvColumnItemList.get(theBeginIndexOfValue); - mvColumnItem.setAggregationType(AggregateType.NONE, true); } } diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java index ec603c0d9eb248..486eeb54811214 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java @@ -25,6 +25,7 @@ import org.apache.doris.catalog.Index; import org.apache.doris.catalog.KeysType; import org.apache.doris.catalog.PartitionType; +import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.ErrorCode; @@ -284,13 +285,22 @@ public void analyze(Analyzer analyzer) throws UserException { } else { for (ColumnDef columnDef : columnDefs) { keyLength += columnDef.getType().getStorageLayoutBytes(); - if ((!columnDef.getType().getPrimitiveType().isFloatingPointType()) - && ((keysColumnNames.size() < FeConstants.shortkey_max_column_count) - || (keyLength < FeConstants.shortkey_maxsize_bytes))) { + if (keysColumnNames.size() >= FeConstants.shortkey_max_column_count + || keyLength > FeConstants.shortkey_maxsize_bytes) { + if (keysColumnNames.size() == 0 + && columnDef.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { + keysColumnNames.add(columnDef.getName()); + } + break; + } + if (columnDef.getType().isFloatingPointType()) { + break; + } + if (columnDef.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { keysColumnNames.add(columnDef.getName()); - } else { break; } + keysColumnNames.add(columnDef.getName()); } // The OLAP table must has at least one short key and the float and double should not be short key. // So the float and double could not be the first column in OLAP table. diff --git a/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java b/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java index fc5523b7d11db0..4898ebfe5627ab 100644 --- a/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java +++ b/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java @@ -19,6 +19,7 @@ import org.apache.doris.catalog.AggregateType; import org.apache.doris.catalog.PrimitiveType; +import org.apache.doris.catalog.Type; /** * This is a result of semantic analysis for AddMaterializedViewClause. @@ -29,7 +30,7 @@ public class MVColumnItem { private String name; // the origin type of slot ref - private PrimitiveType type; + private Type type; private boolean isKey; private AggregateType aggregationType; private boolean isAggregationTypeImplicit; @@ -43,12 +44,12 @@ public String getName() { return name; } - public void setType(PrimitiveType type) { - this.type = type; + public Type getType() { + return type; } - public PrimitiveType getType() { - return type; + public void setType(Type type) { + this.type = type; } public void setIsKey(boolean key) { diff --git a/fe/src/main/java/org/apache/doris/catalog/Catalog.java b/fe/src/main/java/org/apache/doris/catalog/Catalog.java index 773c76ee103008..b04b1621adeee2 100755 --- a/fe/src/main/java/org/apache/doris/catalog/Catalog.java +++ b/fe/src/main/java/org/apache/doris/catalog/Catalog.java @@ -4879,29 +4879,30 @@ public static short calcShortKeyColumnCount(List columns, Map FeConstants.shortkey_maxsize_bytes) { - break; - } + int maxShortKeyColumnCount = Math.min(indexColumns.size(), FeConstants.shortkey_max_column_count); + for (int i = 0; i < maxShortKeyColumnCount; i++) { + Column column = indexColumns.get(i); + shortKeySizeByte += column.getOlapColumnIndexSize(); + if (shortKeySizeByte > FeConstants.shortkey_maxsize_bytes) { if (column.getDataType() == PrimitiveType.VARCHAR) { ++shortKeyColumnCount; - break; } + break; + } + if (column.getType().isFloatingPointType()) { + break; + } + if (column.getDataType() == PrimitiveType.VARCHAR) { ++shortKeyColumnCount; + break; } + ++shortKeyColumnCount; + } + if (shortKeyColumnCount == 0) { + throw new DdlException("The first column could not be float or double type, use decimal instead"); } - // else - // first column type is VARCHAR - // use only first column as shortKey - // do nothing here } // end calc shortKeyColumnCount diff --git a/fe/src/main/java/org/apache/doris/catalog/Type.java b/fe/src/main/java/org/apache/doris/catalog/Type.java index e2d8119d7bce7c..4ba15c133c02f0 100644 --- a/fe/src/main/java/org/apache/doris/catalog/Type.java +++ b/fe/src/main/java/org/apache/doris/catalog/Type.java @@ -1030,4 +1030,12 @@ public Type getNumResultType() { public int getStorageLayoutBytes() { return 0; } + + public int getIndexSize() { + if (this.getPrimitiveType() == PrimitiveType.CHAR) { + return ((ScalarType) this).getLength(); + } else { + return this.getPrimitiveType().getOlapColumnIndexSize(); + } + } } diff --git a/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java b/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java index fdce6b5d7c6c8e..1ed47f58c4c878 100644 --- a/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java +++ b/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java @@ -689,22 +689,18 @@ public void testMVColumnsWithoutOrderbyWithoutAggregationWithFloat(@Injectable S result = columnName3; slotRef4.getColumnName(); result = columnName4; - selectStmt.getResultExprs(); - result = Lists.newArrayList(slotRef1, slotRef2, slotRef3, slotRef4); - slotRef1.getType().getStorageLayoutBytes(); + slotRef1.getType().getIndexSize(); result = 1; slotRef1.getType().getPrimitiveType(); result = PrimitiveType.INT; - slotRef2.getType().getStorageLayoutBytes(); + slotRef2.getType().getIndexSize(); result = 2; slotRef2.getType().getPrimitiveType(); result = PrimitiveType.INT; - slotRef3.getType().getStorageLayoutBytes(); + slotRef3.getType().getIndexSize(); result = 3; - slotRef3.getType().getPrimitiveType(); - result = PrimitiveType.FLOAT; - slotRef4.getType().getPrimitiveType(); - result = PrimitiveType.INT; + slotRef3.getType().isFloatingPointType(); + result = true; selectStmt.getAggInfo(); // return null, so that the mv can be a duplicate mv result = null; } @@ -742,6 +738,104 @@ public void testMVColumnsWithoutOrderbyWithoutAggregationWithFloat(@Injectable S } } + /* + ISSUE: #3811 + */ + @Test + public void testMVColumnsWithoutOrderbyWithoutAggregationWithVarchar(@Injectable SlotRef slotRef1, + @Injectable SlotRef slotRef2, @Injectable SlotRef slotRef3, @Injectable SlotRef slotRef4, + @Injectable TableRef tableRef, @Injectable SelectStmt selectStmt) throws UserException { + SelectList selectList = new SelectList(); + SelectListItem selectListItem1 = new SelectListItem(slotRef1, null); + selectList.addItem(selectListItem1); + SelectListItem selectListItem2 = new SelectListItem(slotRef2, null); + selectList.addItem(selectListItem2); + SelectListItem selectListItem3 = new SelectListItem(slotRef3, null); + selectList.addItem(selectListItem3); + SelectListItem selectListItem4 = new SelectListItem(slotRef4, null); + selectList.addItem(selectListItem4); + + final String columnName1 = "k1"; + final String columnName2 = "k2"; + final String columnName3 = "v1"; + final String columnName4 = "v2"; + + new Expectations() { + { + analyzer.getClusterName(); + result = "default"; + selectStmt.getAggInfo(); + result = null; + selectStmt.getSelectList(); + result = selectList; + selectStmt.getTableRefs(); + result = Lists.newArrayList(tableRef); + selectStmt.getWhereClause(); + result = null; + selectStmt.getHavingPred(); + result = null; + selectStmt.getOrderByElements(); + result = null; + selectStmt.getLimit(); + result = -1; + selectStmt.analyze(analyzer); + slotRef1.getColumnName(); + result = columnName1; + slotRef2.getColumnName(); + result = columnName2; + slotRef3.getColumnName(); + result = columnName3; + slotRef4.getColumnName(); + result = columnName4; + slotRef1.getType().getIndexSize(); + result = 1; + slotRef1.getType().getPrimitiveType(); + result = PrimitiveType.INT; + slotRef2.getType().getIndexSize(); + result = 2; + slotRef2.getType().getPrimitiveType(); + result = PrimitiveType.INT; + slotRef3.getType().getIndexSize(); + result = 3; + slotRef3.getType().getPrimitiveType(); + result = PrimitiveType.VARCHAR; + selectStmt.getAggInfo(); // return null, so that the mv can be a duplicate mv + result = null; + } + }; + + + CreateMaterializedViewStmt createMaterializedViewStmt = new CreateMaterializedViewStmt("test", selectStmt, null); + try { + createMaterializedViewStmt.analyze(analyzer); + Assert.assertEquals(KeysType.DUP_KEYS, createMaterializedViewStmt.getMVKeysType()); + List mvColumns = createMaterializedViewStmt.getMVColumnItemList(); + Assert.assertEquals(4, mvColumns.size()); + MVColumnItem mvColumn0 = mvColumns.get(0); + Assert.assertTrue(mvColumn0.isKey()); + Assert.assertFalse(mvColumn0.isAggregationTypeImplicit()); + Assert.assertEquals(columnName1, mvColumn0.getName()); + Assert.assertEquals(null, mvColumn0.getAggregationType()); + MVColumnItem mvColumn1 = mvColumns.get(1); + Assert.assertTrue(mvColumn1.isKey()); + Assert.assertFalse(mvColumn1.isAggregationTypeImplicit()); + Assert.assertEquals(columnName2, mvColumn1.getName()); + Assert.assertEquals(null, mvColumn1.getAggregationType()); + MVColumnItem mvColumn2 = mvColumns.get(2); + Assert.assertTrue(mvColumn2.isKey()); + Assert.assertFalse(mvColumn2.isAggregationTypeImplicit()); + Assert.assertEquals(columnName3, mvColumn2.getName()); + Assert.assertEquals(null, mvColumn2.getAggregationType()); + MVColumnItem mvColumn3 = mvColumns.get(3); + Assert.assertFalse(mvColumn3.isKey()); + Assert.assertTrue(mvColumn3.isAggregationTypeImplicit()); + Assert.assertEquals(columnName4, mvColumn3.getName()); + Assert.assertEquals(AggregateType.NONE, mvColumn3.getAggregationType()); + } catch (UserException e) { + Assert.fail(e.getMessage()); + } + } + /* ISSUE: #3811 */ @@ -773,12 +867,8 @@ public void testMVColumnsWithFirstFloat(@Injectable SlotRef slotRef1, selectStmt.analyze(analyzer); slotRef1.getColumnName(); result = columnName1; - selectStmt.getResultExprs(); - result = Lists.newArrayList(slotRef1); - slotRef1.getType().getStorageLayoutBytes(); - result = 35; - slotRef1.getType().getPrimitiveType(); - result = PrimitiveType.FLOAT; + slotRef1.getType().isFloatingPointType(); + result = true; selectStmt.getAggInfo(); // return null, so that the mv can be a duplicate mv result = null; } @@ -794,6 +884,62 @@ public void testMVColumnsWithFirstFloat(@Injectable SlotRef slotRef1, } } + /* + ISSUE: #3811 + */ + @Test + public void testMVColumnsWithFirstVarchar(@Injectable SlotRef slotRef1, + @Injectable TableRef tableRef, @Injectable SelectStmt selectStmt) throws UserException { + SelectList selectList = new SelectList(); + SelectListItem selectListItem1 = new SelectListItem(slotRef1, null); + selectList.addItem(selectListItem1); + + final String columnName1 = "k1"; + + new Expectations() { + { + analyzer.getClusterName(); + result = "default"; + selectStmt.getAggInfo(); + result = null; + selectStmt.getSelectList(); + result = selectList; + selectStmt.getTableRefs(); + result = Lists.newArrayList(tableRef); + selectStmt.getWhereClause(); + result = null; + selectStmt.getHavingPred(); + result = null; + selectStmt.getOrderByElements(); + result = null; + selectStmt.getLimit(); + result = -1; + selectStmt.analyze(analyzer); + slotRef1.getColumnName(); + result = columnName1; + slotRef1.getType().getPrimitiveType(); + result = PrimitiveType.VARCHAR; + } + }; + + + CreateMaterializedViewStmt createMaterializedViewStmt = new CreateMaterializedViewStmt("test", selectStmt, null); + try { + createMaterializedViewStmt.analyze(analyzer); + Assert.assertEquals(KeysType.DUP_KEYS, createMaterializedViewStmt.getMVKeysType()); + List mvColumns = createMaterializedViewStmt.getMVColumnItemList(); + Assert.assertEquals(1, mvColumns.size()); + MVColumnItem mvColumn0 = mvColumns.get(0); + Assert.assertTrue(mvColumn0.isKey()); + Assert.assertFalse(mvColumn0.isAggregationTypeImplicit()); + Assert.assertEquals(columnName1, mvColumn0.getName()); + Assert.assertEquals(null, mvColumn0.getAggregationType()); + } catch (UserException e) { + Assert.fail(e.getMessage()); + } + } + + @Test public void testMVColumns(@Injectable SlotRef slotRef1, @Injectable SlotRef slotRef2, From eaa8c7dee01a7ede52a78270c51aecffa7259685 Mon Sep 17 00:00:00 2001 From: emmymiao87 <522274284@qq.com> Date: Thu, 11 Jun 2020 10:51:30 +0800 Subject: [PATCH 07/13] Remove unused Change-Id: Ia4d2a9bfb1b4321dc3ed8e2dd243d9a4fd2dbf69 --- .../org/apache/doris/analysis/CreateMaterializedViewStmt.java | 1 - 1 file changed, 1 deletion(-) diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java index cb65fb8352f0f0..48b3bf4b037926 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java @@ -69,7 +69,6 @@ public class CreateMaterializedViewStmt extends DdlStmt { private String baseIndexName; private String dbName; private KeysType mvKeysType = KeysType.DUP_KEYS; - private int shortKeyColumnCount; public CreateMaterializedViewStmt(String mvName, SelectStmt selectStmt, Map properties) { this.mvName = mvName; From 269a501f872203800c916b924c1d75b4799c7389 Mon Sep 17 00:00:00 2001 From: emmymiao87 <522274284@qq.com> Date: Fri, 12 Jun 2020 11:45:21 +0800 Subject: [PATCH 08/13] Change comment Change-Id: I1d13600a39d59cf93feb42df85c1089ca1029615 --- .../java/org/apache/doris/alter/MaterializedViewHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java index f4fcc06334b4be..38b3decbb6ea14 100644 --- a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java +++ b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java @@ -593,7 +593,7 @@ public List checkAndPrepareMaterializedView(AddRollupClause addRollupCla column.setIsKey(true); } if (theBeginIndexOfValue == 0) { - throw new DdlException("The first column could not be float or double, use decimal instead."); + throw new DdlException("The first column could not be float or double"); } // Supplement value of MV columns for (; theBeginIndexOfValue < rollupSchema.size(); theBeginIndexOfValue++) { From 9317574ab0042319b4ace0ec5f8006682f515e14 Mon Sep 17 00:00:00 2001 From: emmymiao87 <522274284@qq.com> Date: Mon, 15 Jun 2020 10:32:21 +0800 Subject: [PATCH 09/13] Remove unused import Change-Id: Id647f940b81ebfc27e18ef83e5275af7177bf926 --- fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java | 1 - 1 file changed, 1 deletion(-) diff --git a/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java b/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java index 4898ebfe5627ab..e92bd4e9c1e653 100644 --- a/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java +++ b/fe/src/main/java/org/apache/doris/analysis/MVColumnItem.java @@ -18,7 +18,6 @@ package org.apache.doris.analysis; import org.apache.doris.catalog.AggregateType; -import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.catalog.Type; /** From 37d2b2a5c03d712a399b5b6c6ec9406f3236ace5 Mon Sep 17 00:00:00 2001 From: emmymiao87 <522274284@qq.com> Date: Mon, 15 Jun 2020 14:46:57 +0800 Subject: [PATCH 10/13] Fix bug Change-Id: I4d560427156855f28598ed089048006ea8d685c4 --- fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java index 486eeb54811214..2c9ee25404ea7d 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java @@ -284,7 +284,7 @@ public void analyze(Analyzer analyzer) throws UserException { keysDesc = new KeysDesc(KeysType.AGG_KEYS, keysColumnNames); } else { for (ColumnDef columnDef : columnDefs) { - keyLength += columnDef.getType().getStorageLayoutBytes(); + keyLength += columnDef.getType().getIndexSize(); if (keysColumnNames.size() >= FeConstants.shortkey_max_column_count || keyLength > FeConstants.shortkey_maxsize_bytes) { if (keysColumnNames.size() == 0 From 2d3babab4a34f99b9e7695831faf8c28d383ad18 Mon Sep 17 00:00:00 2001 From: emmymiao87 <522274284@qq.com> Date: Mon, 15 Jun 2020 17:33:51 +0800 Subject: [PATCH 11/13] Fix bug Change-Id: Iccfce893d120506667c55444a11841b91a1d322b --- .../apache/doris/analysis/CreateMaterializedViewStmt.java | 5 ++--- .../java/org/apache/doris/analysis/CreateTableStmt.java | 5 ++--- fe/src/main/java/org/apache/doris/catalog/Catalog.java | 4 ++-- .../main/java/org/apache/doris/catalog/PrimitiveType.java | 8 ++++++-- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java index 48b3bf4b037926..90a2678a5371a0 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java @@ -19,7 +19,6 @@ import org.apache.doris.catalog.AggregateType; import org.apache.doris.catalog.KeysType; -import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.ErrorCode; @@ -300,7 +299,7 @@ private void supplyOrderColumn() throws AnalysisException { keySizeByte += column.getType().getIndexSize(); if (theBeginIndexOfValue + 1 > FeConstants.shortkey_max_column_count || keySizeByte > FeConstants.shortkey_maxsize_bytes) { - if (theBeginIndexOfValue == 0 && column.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { + if (theBeginIndexOfValue == 0 && column.getType().getPrimitiveType().isCharFamily()) { column.setIsKey(true); theBeginIndexOfValue++; } @@ -309,7 +308,7 @@ private void supplyOrderColumn() throws AnalysisException { if (column.getType().isFloatingPointType()) { break; } - if (column.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { + if (column.getType().getPrimitiveType().isCharFamily()) { column.setIsKey(true); theBeginIndexOfValue++; break; diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java index 2c9ee25404ea7d..7e3b8071940a7a 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java @@ -25,7 +25,6 @@ import org.apache.doris.catalog.Index; import org.apache.doris.catalog.KeysType; import org.apache.doris.catalog.PartitionType; -import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.ErrorCode; @@ -288,7 +287,7 @@ public void analyze(Analyzer analyzer) throws UserException { if (keysColumnNames.size() >= FeConstants.shortkey_max_column_count || keyLength > FeConstants.shortkey_maxsize_bytes) { if (keysColumnNames.size() == 0 - && columnDef.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { + && columnDef.getType().getPrimitiveType().isCharFamily()) { keysColumnNames.add(columnDef.getName()); } break; @@ -296,7 +295,7 @@ public void analyze(Analyzer analyzer) throws UserException { if (columnDef.getType().isFloatingPointType()) { break; } - if (columnDef.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { + if (columnDef.getType().getPrimitiveType().isCharFamily()) { keysColumnNames.add(columnDef.getName()); break; } diff --git a/fe/src/main/java/org/apache/doris/catalog/Catalog.java b/fe/src/main/java/org/apache/doris/catalog/Catalog.java index b04b1621adeee2..706332d52cacd4 100755 --- a/fe/src/main/java/org/apache/doris/catalog/Catalog.java +++ b/fe/src/main/java/org/apache/doris/catalog/Catalog.java @@ -4886,7 +4886,7 @@ public static short calcShortKeyColumnCount(List columns, Map FeConstants.shortkey_maxsize_bytes) { - if (column.getDataType() == PrimitiveType.VARCHAR) { + if (column.getDataType().isCharFamily()) { ++shortKeyColumnCount; } break; @@ -4894,7 +4894,7 @@ public static short calcShortKeyColumnCount(List columns, Map Date: Mon, 15 Jun 2020 18:09:07 +0800 Subject: [PATCH 12/13] Fix bug Change-Id: Ia88fa8cfa70661328300864054962931a8c0e225 --- .../org/apache/doris/analysis/CreateMaterializedViewStmt.java | 3 ++- .../main/java/org/apache/doris/analysis/CreateTableStmt.java | 3 ++- fe/src/main/java/org/apache/doris/catalog/Catalog.java | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java index 90a2678a5371a0..6f0ff6f75b06c9 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java @@ -19,6 +19,7 @@ import org.apache.doris.catalog.AggregateType; import org.apache.doris.catalog.KeysType; +import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.ErrorCode; @@ -308,7 +309,7 @@ private void supplyOrderColumn() throws AnalysisException { if (column.getType().isFloatingPointType()) { break; } - if (column.getType().getPrimitiveType().isCharFamily()) { + if (column.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { column.setIsKey(true); theBeginIndexOfValue++; break; diff --git a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java index 7e3b8071940a7a..ded7c78837e970 100644 --- a/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java +++ b/fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java @@ -25,6 +25,7 @@ import org.apache.doris.catalog.Index; import org.apache.doris.catalog.KeysType; import org.apache.doris.catalog.PartitionType; +import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.ErrorCode; @@ -295,7 +296,7 @@ public void analyze(Analyzer analyzer) throws UserException { if (columnDef.getType().isFloatingPointType()) { break; } - if (columnDef.getType().getPrimitiveType().isCharFamily()) { + if (columnDef.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { keysColumnNames.add(columnDef.getName()); break; } diff --git a/fe/src/main/java/org/apache/doris/catalog/Catalog.java b/fe/src/main/java/org/apache/doris/catalog/Catalog.java index 706332d52cacd4..da9906c3afc1aa 100755 --- a/fe/src/main/java/org/apache/doris/catalog/Catalog.java +++ b/fe/src/main/java/org/apache/doris/catalog/Catalog.java @@ -4894,7 +4894,7 @@ public static short calcShortKeyColumnCount(List columns, Map Date: Tue, 16 Jun 2020 16:09:09 +0800 Subject: [PATCH 13/13] Fix ut Change-Id: I88e36a2379ae2e0659358bb5ba59d93bccfe37f1 --- .../doris/alter/MaterializedViewHandler.java | 2 +- .../CreateMaterializedViewStmtTest.java | 18 +++++++----------- .../apache/doris/planner/QueryPlanTest.java | 6 +++--- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java index 38b3decbb6ea14..8626374bf9346e 100644 --- a/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java +++ b/fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java @@ -576,7 +576,7 @@ public List checkAndPrepareMaterializedView(AddRollupClause addRollupCla keySizeByte += column.getType().getIndexSize(); if (theBeginIndexOfValue + 1 > FeConstants.shortkey_max_column_count || keySizeByte > FeConstants.shortkey_maxsize_bytes) { - if (theBeginIndexOfValue == 0 && column.getType().getPrimitiveType() == PrimitiveType.VARCHAR) { + if (theBeginIndexOfValue == 0 && column.getType().getPrimitiveType().isCharFamily()) { column.setIsKey(true); theBeginIndexOfValue++; } diff --git a/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java b/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java index 1ed47f58c4c878..1a778fbdd19001 100644 --- a/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java +++ b/fe/src/test/java/org/apache/doris/analysis/CreateMaterializedViewStmtTest.java @@ -585,24 +585,20 @@ public void testMVColumnsWithoutOrderbyWithoutAggregation(@Injectable SlotRef sl result = columnName3; slotRef4.getColumnName(); result = columnName4; - selectStmt.getResultExprs(); - result = Lists.newArrayList(slotRef1, slotRef2, slotRef3, slotRef4); - slotRef1.getType().getStorageLayoutBytes(); - result = 35; + slotRef1.getType().getIndexSize(); + result = 34; slotRef1.getType().getPrimitiveType(); result = PrimitiveType.INT; - slotRef2.getType().getStorageLayoutBytes(); - result = 2; + slotRef2.getType().getIndexSize(); + result = 1; slotRef2.getType().getPrimitiveType(); result = PrimitiveType.INT; - slotRef3.getType().getStorageLayoutBytes(); - result = 3; + slotRef3.getType().getIndexSize(); + result = 1; slotRef3.getType().getPrimitiveType(); result = PrimitiveType.INT; - slotRef4.getType().getStorageLayoutBytes(); + slotRef4.getType().getIndexSize(); result = 4; - slotRef4.getType().getPrimitiveType(); - result = PrimitiveType.INT; selectStmt.getAggInfo(); // return null, so that the mv can be a duplicate mv result = null; } diff --git a/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java index dc05ec54ce1a6e..9195490e1957c2 100644 --- a/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java +++ b/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java @@ -69,9 +69,9 @@ public static void beforeClass() throws Exception { Catalog.getCurrentCatalog().createDb(createDbStmt); createTable("create table test.test1\n" + - "(\n" + - " query_id varchar(48) comment \"Unique query id\",\n" + - " time datetime not null comment \"Query start time\",\n" + + "(\n" + + " time datetime not null comment \"Query start time\",\n" + + " query_id varchar(48) comment \"Unique query id\",\n" + " client_ip varchar(32) comment \"Client IP\",\n" + " user varchar(64) comment \"User name\",\n" + " db varchar(96) comment \"Database of this query\",\n" +