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/02/11 13:03:26 UTC
[GitHub] [incubator-doris] yangzhg opened a new pull request #2883: add
column type check when create bitmap index using create index clause
yangzhg opened a new pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883
1. add column AggregateType check when create bitmap index using create index clause
2. support more data type in bitmap index
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman merged pull request #2883: add column
type check when create bitmap index using create index clause
Posted by GitBox <gi...@apache.org>.
morningman merged pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yangzhg commented on a change in pull request
#2883: add column type check when create bitmap index using create index
clause
Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883#discussion_r377679923
##########
File path: fe/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -200,12 +202,39 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
}
Set<String> idxSet = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
idxSet.addAll(idx.getColumns());
- if (newColset.equals(idxSet)) {
- throw new AnalysisException("index for columns (" + String
- .join(",", indexDef.getColumns()) + " ) already exist.");
+ if (newColset.equals(idxSet)) {
+ throw new AnalysisException("index for columns (" + String
+ .join(",", indexDef.getColumns()) + " ) already exist.");
+ }
+ }
+ OlapTable olapTable = (OlapTable) table;
+ for (String col : indexDef.getColumns()) {
+ for (Column c : olapTable.getFullSchema()) {
Review comment:
the colum name from create index clause may not the same case with `olapTable.getColumn ()` so i must use case `equalsIgnoreCase` to find
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#2883: add column type check when create bitmap index using create index
clause
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883#discussion_r377642101
##########
File path: fe/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -200,12 +202,39 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
}
Set<String> idxSet = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
idxSet.addAll(idx.getColumns());
- if (newColset.equals(idxSet)) {
- throw new AnalysisException("index for columns (" + String
- .join(",", indexDef.getColumns()) + " ) already exist.");
+ if (newColset.equals(idxSet)) {
+ throw new AnalysisException("index for columns (" + String
+ .join(",", indexDef.getColumns()) + " ) already exist.");
+ }
+ }
+ OlapTable olapTable = (OlapTable) table;
+ for (String col : indexDef.getColumns()) {
+ for (Column c : olapTable.getFullSchema()) {
Review comment:
And the following type checking logic looks same as in `CreateTableStmt`.
So I think they can be extract to a method?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wutiangan commented on a change in pull request
#2883: add column type check when create bitmap index using create index
clause
Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883#discussion_r377632933
##########
File path: fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java
##########
@@ -412,9 +412,13 @@ public void analyze(Analyzer analyzer) throws UserException {
// key columns and none/replace aggregate non-key columns support
if (indexDef.getIndexType() == IndexDef.IndexType.BITMAP) {
- if (!(colType == PrimitiveType.TINYINT || colType == PrimitiveType.SMALLINT
- || colType == PrimitiveType.INT || colType == PrimitiveType.BIGINT ||
- colType == PrimitiveType.CHAR || colType == PrimitiveType.VARCHAR)) {
+ if (!(colType == PrimitiveType.TINYINT || colType == PrimitiveType.SMALLINT ||
Review comment:
use isNumericType(), not list all type
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#2883: add column type check when create bitmap index using create index
clause
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883#discussion_r378263787
##########
File path: fe/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -200,12 +202,39 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
}
Set<String> idxSet = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
idxSet.addAll(idx.getColumns());
- if (newColset.equals(idxSet)) {
- throw new AnalysisException("index for columns (" + String
- .join(",", indexDef.getColumns()) + " ) already exist.");
+ if (newColset.equals(idxSet)) {
+ throw new AnalysisException("index for columns (" + String
+ .join(",", indexDef.getColumns()) + " ) already exist.");
+ }
+ }
+ OlapTable olapTable = (OlapTable) table;
+ for (String col : indexDef.getColumns()) {
+ for (Column c : olapTable.getFullSchema()) {
Review comment:
This method is case insensitive `olapTable.getColumn()`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] kangkaisen commented on a change in pull request
#2883: add column type check when create bitmap index using create index
clause
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883#discussion_r378019553
##########
File path: fe/src/main/java/org/apache/doris/analysis/IndexDef.java
##########
@@ -118,4 +121,32 @@ public String getComment() {
public enum IndexType {
BITMAP,
}
+ public void checkColumn(Column column) throws AnalysisException {
+ if (indexType == IndexType.BITMAP) {
+ String indexColName = column.getName();
+ PrimitiveType colType = column.getDataType();
+ if (!(colType.isDateType() || colType.isDecimalType() || colType.isFixedPointType() ||
+ colType.isStringType() || colType == PrimitiveType.BOOLEAN)) {
+ throw new AnalysisException(colType + " is not supported in bitmap index. "
+ + "invalid column: " + indexColName);
+ } else if (!(column.isKey()
+ || column.getAggregationType() == AggregateType.NONE
+ || column.getAggregationType() == AggregateType.REPLACE
+ || column.getAggregationType() == AggregateType.REPLACE_IF_NOT_NULL)){
+ // althrough the implemention supports bf for replace non-key column,
Review comment:
`althrough the implemention supports bf for replace non-key column`
What's meaning of `bf`? This comment seems conflict with `bitmap 索引能够应用在 `Duplicate` 数据模型的所有列和 `Aggregate`, `Uniq` 模型的key列上和 `REPLACE` 的value列上。`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wutiangan commented on a change in pull request
#2883: add column type check when create bitmap index using create index
clause
Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883#discussion_r377629893
##########
File path: docs/documentation/en/administrator-guide/alter-table/alter-table-bitmap-index_EN.md
##########
@@ -56,7 +56,10 @@ Please refer to [Scheam Change](alter-table-schema-change_EN.md#Cancel Job)
## Notice
* Currently only index of bitmap type is supported.
* The bitmap index is only created on a single column.
-* Bitmap indexes can be applied to all columns of the `Duplicate` data model and key columns of the `Aggregate` and `Uniq` models.
+* Bitmap indexes can be applied to all columns of the `Duplicate` data model and key columns or `REPLACE` value columns
+ of the
+ `Aggregate` and
+ `Uniq` models.
Review comment:
change many lines to on line.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yangzhg commented on a change in pull request
#2883: add column type check when create bitmap index using create index
clause
Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883#discussion_r378023023
##########
File path: fe/src/main/java/org/apache/doris/analysis/IndexDef.java
##########
@@ -118,4 +121,32 @@ public String getComment() {
public enum IndexType {
BITMAP,
}
+ public void checkColumn(Column column) throws AnalysisException {
+ if (indexType == IndexType.BITMAP) {
+ String indexColName = column.getName();
+ PrimitiveType colType = column.getDataType();
+ if (!(colType.isDateType() || colType.isDecimalType() || colType.isFixedPointType() ||
+ colType.isStringType() || colType == PrimitiveType.BOOLEAN)) {
+ throw new AnalysisException(colType + " is not supported in bitmap index. "
+ + "invalid column: " + indexColName);
+ } else if (!(column.isKey()
+ || column.getAggregationType() == AggregateType.NONE
+ || column.getAggregationType() == AggregateType.REPLACE
+ || column.getAggregationType() == AggregateType.REPLACE_IF_NOT_NULL)){
+ // althrough the implemention supports bf for replace non-key column,
Review comment:
its a typo i will remove it
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yangzhg commented on a change in pull request
#2883: add column type check when create bitmap index using create index
clause
Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883#discussion_r377677716
##########
File path: fe/src/main/java/org/apache/doris/analysis/CreateTableStmt.java
##########
@@ -412,9 +412,13 @@ public void analyze(Analyzer analyzer) throws UserException {
// key columns and none/replace aggregate non-key columns support
if (indexDef.getIndexType() == IndexDef.IndexType.BITMAP) {
- if (!(colType == PrimitiveType.TINYINT || colType == PrimitiveType.SMALLINT
- || colType == PrimitiveType.INT || colType == PrimitiveType.BIGINT ||
- colType == PrimitiveType.CHAR || colType == PrimitiveType.VARCHAR)) {
+ if (!(colType == PrimitiveType.TINYINT || colType == PrimitiveType.SMALLINT ||
Review comment:
it dose not support double and float and double， so cannot use isNumericType()
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#2883: add column type check when create bitmap index using create index
clause
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2883: add column type check when create bitmap index using create index clause
URL: https://github.com/apache/incubator-doris/pull/2883#discussion_r377640999
##########
File path: fe/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -200,12 +202,39 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
}
Set<String> idxSet = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
idxSet.addAll(idx.getColumns());
- if (newColset.equals(idxSet)) {
- throw new AnalysisException("index for columns (" + String
- .join(",", indexDef.getColumns()) + " ) already exist.");
+ if (newColset.equals(idxSet)) {
+ throw new AnalysisException("index for columns (" + String
+ .join(",", indexDef.getColumns()) + " ) already exist.");
+ }
+ }
+ OlapTable olapTable = (OlapTable) table;
+ for (String col : indexDef.getColumns()) {
+ for (Column c : olapTable.getFullSchema()) {
Review comment:
You can just use `olapTable.getColumn()` to get the column, no need to iterate all columns
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org