[GitHub] [incubator-doris] HangyuanLiu commented on a change in pull request #4247: Unified the grammer style of create/drop bitmap and bloomfilter index

[GitBox]

HangyuanLiu commented on a change in pull request #4247:
URL: https://github.com/apache/incubator-doris/pull/4247#discussion_r468294969



##########
File path: 
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
##########
@@ -1160,6 +1160,326 @@ private void createJob(long dbId, OlapTable olapTable, 
Map<Long, LinkedList<Colu
         LOG.info("finished to create schema change job: {}", 
schemaChangeJob.getJobId());
     }
 
+    private void createBfJob(long dbId, OlapTable olapTable, Map<Long, 
LinkedList<Column>> indexSchemaMap,

Review comment:
       Can you reuse the old logic in createJob

##########
File path: docs/zh-CN/sql-reference/sql-statements/Data Definition/CREATE 
INDEX.md
##########
@@ -31,14 +31,18 @@ under the License.
     该语句用于创建索引
     语法：
         CREATE INDEX index_name ON table_name (column [, ...],) [USING BITMAP] 
[COMMENT'balabala'];
+        CREATE INDEX index_name ON table_name (column [, ...],) [USING 
BLOOMFILTER] [COMMENT'balabala'] PROPERTIES ('bloom_filter_fpp'='0.05');
     注意：
-        1. 目前只支持bitmap 索引
+        1. 目前支持bitmap,bloomfilter 索引
         2. BITMAP 索引仅在单列上创建
+       3. BLOOMFILTER索引可以在多列上创建，当列为空时表示删除索引；属性bloom_filter_fpp表示期望误差率

Review comment:
       Indent to index 2. 

##########
File path: 
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
##########
@@ -1438,9 +1758,20 @@ public void process(List<AlterClause> alterClauses, 
String clusterName, Database
                 // modify table properties
                 // do nothing, properties are already in propertyMap
             } else if (alterClause instanceof CreateIndexClause) {
-                processAddIndex((CreateIndexClause) alterClause, olapTable, 
newIndexes);
+                CreateIndexClause clause = (CreateIndexClause) alterClause;
+                processAddIndex(clause, olapTable, newIndexes);
+                if (clause.getIndexDef().getIndexType() == 
IndexDef.IndexType.BLOOMFILTER) {
+                    createBfJob(db.getId(), olapTable, indexSchemaMap, 
propertyMap, newIndexes, false);
+                    return;
+                }
             } else if (alterClause instanceof DropIndexClause) {
-                processDropIndex((DropIndexClause) alterClause, olapTable, 
newIndexes);
+                DropIndexClause clause = (DropIndexClause) alterClause;
+                if (getIndexTypeByName(clause, olapTable) == 
IndexDef.IndexType.BLOOMFILTER) {
+                    createBfJob(db.getId(), olapTable, indexSchemaMap, 
propertyMap, newIndexes, true);

Review comment:
       I see, but createBfJob is ambiguous 

##########
File path: 
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
##########
@@ -1729,6 +2075,9 @@ private void processDropIndex(DropIndexClause 
alterClause, OlapTable olapTable,
                 break;
             }
         }
+        if (olapTable.getBfName() == indexName) {
+            found = new Index();

Review comment:
       Please add a comment to explain why a new Index was created here




----------------------------------------------------------------
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org