You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/06/09 16:11:33 UTC
[GitHub] [incubator-doris] EmmyMiao87 opened a new pull request #3812: Forbidden float column in short key
EmmyMiao87 opened a new pull request #3812:
URL: https://github.com/apache/incubator-doris/pull/3812
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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] EmmyMiao87 commented on a change in pull request #3812: Forbidden float column in short key
Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on a change in pull request #3812:
URL: https://github.com/apache/incubator-doris/pull/3812#discussion_r437823982
##########
File path: fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
##########
@@ -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())
Review comment:
Why?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #3812: Forbidden float column in short key
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3812:
URL: https://github.com/apache/incubator-doris/pull/3812#discussion_r438196277
##########
File path: fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
##########
@@ -68,9 +69,9 @@
private String baseIndexName;
private String dbName;
private KeysType mvKeysType = KeysType.DUP_KEYS;
+ private int shortKeyColumnCount;
Review comment:
Not use it?
##########
File path: fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
##########
@@ -226,49 +224,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;
- }
- 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 ((i + 1) <= FeConstants.shortkey_max_column_count
- || keyStorageLayoutBytes < FeConstants.shortkey_maxsize_bytes) {
- mvColumnItem.setIsKey(true);
- } else {
- mvColumnItem.setAggregationType(AggregateType.NONE, true);
- }
- }
+ supplyShortKey();
Review comment:
```suggestion
selectShortKey();
```
##########
File path: fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
##########
@@ -554,28 +554,42 @@ private RollupJobV2 createMaterializedViewJob(String mvName, String baseIndexNam
} 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");
}
- 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) {
- 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.");
Review comment:
`use decimal instead` can be removed. Because user has no choice.
##########
File path: fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
##########
@@ -554,28 +554,42 @@ private RollupJobV2 createMaterializedViewJob(String mvName, String baseIndexNam
} 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");
}
- 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) {
- 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)) {
Review comment:
```suggestion
|| (keyStorageLayoutBytes > FeConstants.shortkey_maxsize_bytes)) {
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] EmmyMiao87 merged pull request #3812: Forbidden float column in short key
Posted by GitBox <gi...@apache.org>.
EmmyMiao87 merged pull request #3812:
URL: https://github.com/apache/incubator-doris/pull/3812
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #3812: Forbidden float column in short key
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3812:
URL: https://github.com/apache/incubator-doris/pull/3812#discussion_r437468475
##########
File path: fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
##########
@@ -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.");
Review comment:
```suggestion
throw new AnalysisException("Float or double can not used as a sort key, use decimal instead.");
```
##########
File path: fe/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
##########
@@ -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())
Review comment:
We should be break if we met a floating point type column. The following case will get wrong result in your logic:
`k1 int, k2 float, k3 int`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] EmmyMiao87 commented on pull request #3812: Forbidden float column in short key
Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on pull request #3812:
URL: https://github.com/apache/incubator-doris/pull/3812#issuecomment-641681154
> Create table will forbidden float/double
Already forbidden
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] chaoyli commented on pull request #3812: Forbidden float column in short key
Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #3812:
URL: https://github.com/apache/incubator-doris/pull/3812#issuecomment-641309914
Create table will forbidden float/double
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org