[fix][dingo-calcite] Add sql syntax verification

dingodb · Jan 8, 2025 · d55abd9 · d55abd9
1 parent a54de00
commit d55abd9
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 23 deletions.
diff --git a/dingo-calcite/src/main/codegen/includes/AlterTable.ftl b/dingo-calcite/src/main/codegen/includes/AlterTable.ftl
@@ -423,6 +423,7 @@ SqlAlterTable addConstraint(Span s, String scope, SqlIdentifier id): {
      }
     |
       <UNIQUE> [<KEY>] [<INDEX>] { index = getNextToken().image; }
+      [ indexTypeName() ]
       columnList = ParenthesizedSimpleIdentifierList()
       {
         return new SqlAlterAddIndex(

diff --git a/dingo-calcite/src/main/codegen/includes/parserImpls.ftl b/dingo-calcite/src/main/codegen/includes/parserImpls.ftl
@@ -225,6 +225,7 @@ void TableElement(List<SqlNode> list) :
     |
         <INDEX> { s.add(this); }
         { index = getNextToken().image; }
+        [ indexTypeName() ]
         (
             <VECTOR>
             { indexType = "vector"; }
@@ -261,7 +262,11 @@ void TableElement(List<SqlNode> list) :
         }
     |
         <KEY> { s.add(this); } name = SimpleIdentifier()
+        [ indexTypeName() ]
         columnList = ParenthesizedSimpleIdentifierList()
+        prop = indexOption()
+        [ indexAlg = indexAlg()]
+        [ indexLockOpt = indexLockOpt()]
         {
             index = name.getSimple();
             list.add(new SqlIndexDeclaration(s.end(this), index, columnList, withColumnList, properties,
@@ -275,10 +280,10 @@ void TableElement(List<SqlNode> list) :
           <INDEX>
         )?
         [ name = SimpleIdentifier() ]
+        [ indexTypeName() ]
         columnList = ParenthesizedSimpleIdentifierList() {
               list.add(new DingoSqlKeyConstraint(s.end(columnList), name, columnList));
         }
-        [<USING> <IDENTIFIER> ]
         [
             <REPLICA> <EQ> {replica = Integer.parseInt(getNextToken().image);}
         ]
@@ -291,6 +296,7 @@ void TableElement(List<SqlNode> list) :
             list.add(SqlDdlNodes.primary(s.end(columnList), name, columnList));
         }
     |
+      [<CONSTRAINT>]
       <FOREIGN><KEY> [ name = SimpleIdentifier() ]
       columnList = ParenthesizedSimpleIdentifierList()
       <REFERENCES> refTable = CompoundIdentifier() refColumnList = ParenthesizedSimpleIdentifierList()
@@ -307,6 +313,26 @@ void TableElement(List<SqlNode> list) :
     )
 }
 
+String indexTypeName(): {
+  String indexType = null;
+} {
+  (
+   <USING>
+   |
+   <TYPE>
+  )
+  (
+   <BTREE> { indexType = "btree";}
+   |
+   <RTREE> { indexType = "rtree"; }
+   |
+   <HASH> { indexType = "hash";}
+  )
+  {
+    return indexType;
+  }
+}
+
 ColumnOption parseColumnOption(): {
    ColumnOption colOpt = new ColumnOption();
    SqlNode e = null;
@@ -349,7 +375,7 @@ ColumnOption parseColumnOption(): {
          |
            <COLLATE> { String collate = strIdent(); colOpt.collate=collate; }
          |
-           <COLUMN_FORMAT> (<FIXED> {colOpt.columnFormat="fixed";}|<DYNAMIC> {colOpt.columnFormat="dynamic";}|<DEFAULT_> {colOpt.columnFormat="default";}) 
+           <COLUMN_FORMAT> (<FIXED> {colOpt.columnFormat="fixed";}|<DYNAMIC> {colOpt.columnFormat="dynamic";}|<DEFAULT_> {colOpt.columnFormat="default";})
          |
           <ON> <UPDATE> <CURRENT_TIMESTAMP>
          |
@@ -377,7 +403,7 @@ ColumnOption parseColumnOption(): {
           <CONSTRAINT>  [name = SimpleIdentifier()] <CHECK> <LPAREN>
              checkExpr = Expression(ExprContext.ACCEPT_SUB_QUERY)
                     <RPAREN> [<NOT> {constraintNot=true;}] [(<ENFORCED> {constraintOpt="enforced";}|<NULL>{constraintOpt="null";})]
-      )*  
+      )*
       {
             if (e == null) {
                 strategy = colOpt.nullable ? ColumnStrategy.NULLABLE
@@ -870,15 +896,12 @@ String indexLockOpt(): {
 
 Properties indexOption(): {
   Properties prop = new Properties();
+  String indexType = "lsm";
 } {
    (
     <KEY_BLOCK_SIZE> [<EQ>] <UNSIGNED_INTEGER_LITERAL> { prop.put("key_block_size", Integer.parseInt(this.token.image));}
     |
-    <RTREE> { prop.put("indexType", "bdb"); }
-    |
-    <BTREE> { prop.put("indexType", "bdb"); }
-    |
-    <HASH> { prop.put("indexType", "hash"); }
+    indexType = indexTypeName() { prop.put("indexType", indexType);}
     |
     <WITH> <PARSER> { prop.put("parser", strIdent()); }
     |

diff --git a/dingo-calcite/src/main/java/io/dingodb/calcite/grammar/ddl/ColumnOption.java b/dingo-calcite/src/main/java/io/dingodb/calcite/grammar/ddl/ColumnOption.java
@@ -22,7 +22,7 @@
 import org.apache.calcite.sql.SqlNodeList;
 
 public class ColumnOption {
-    public boolean nullable;
+    public boolean nullable = true;
     public boolean autoIncrement;
     public boolean primaryKey;
     public boolean clustered = true;

diff --git a/dingo-calcite/src/test/java/io/dingodb/calcite/SqlSyntaxVerification.java b/dingo-calcite/src/test/java/io/dingodb/calcite/SqlSyntaxVerification.java
@@ -25,6 +25,7 @@
 import io.dingodb.calcite.grammar.ddl.SqlAlterDropConstraint;
 import io.dingodb.calcite.grammar.ddl.SqlAlterDropForeign;
 import io.dingodb.calcite.grammar.ddl.SqlAlterModifyColumn;
+import io.dingodb.calcite.grammar.ddl.SqlAlterTable;
 import io.dingodb.calcite.grammar.ddl.SqlCreateIndex;
 import io.dingodb.calcite.grammar.ddl.SqlCreateTenant;
 import io.dingodb.calcite.grammar.ddl.SqlCreateUser;
@@ -315,7 +316,7 @@ public void sqlAlterChangeColumn() {
 
     @Test
     public void sqlCreateIndex() {
-        String sql = "create fulltext index ix on tx(col1(10) asc, col2(20)) btree comment 'commitsss' "
+        String sql = "create fulltext index ix on tx(col1(10) asc, col2(20)) using btree comment 'commitsss' "
             + "algorithm=inplace "
             + "lock =none";
         SqlParser parser = SqlParser.create(sql, PARSER_CONFIG);
@@ -332,7 +333,7 @@ public void sqlCreateIndex() {
 
     @Test
     public void sqlAlterAddFulltextKey() {
-        String sql = "alter table t1 add fulltext key ix(age) btree comment 'commitsss' algorithm=inplace lock=none";
+        String sql = "alter table t1 add fulltext key ix(age) using btree comment 'commitsss' algorithm=inplace lock=none";
         SqlParser parser = SqlParser.create(sql, PARSER_CONFIG);
         try {
             SqlNode sqlNode = parser.parseStmt();
@@ -383,4 +384,45 @@ public void sqlAlterAddIndex() {
         }
     }
 
+    @Test
+    public void sqlIndexTypeOpt() {
+        String sql = "CREATE TABLE `roles_txnlsm` (\n" +
+            "\t`username` varchar(50) NOT NULL,\n" +
+            "\t`role` varchar(50) NOT NULL,\n" +
+            "\tUNIQUE INDEX `idx_user_role` (`username` ASC, `role` ASC) USING BTREE\n" +
+            ") engine=TXN_LSM";
+        SqlParser parser = SqlParser.create(sql, PARSER_CONFIG);
+        try {
+            SqlNode sqlNode = parser.parseStmt();
+            assert sqlNode instanceof SqlCreateTable;
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    @Test
+    public void sqlConstraintForeign() {
+        String sql = "create table t1(id int,age int,name varchar(20),primary key(id), "
+           + "constraint foreign key n1(col1,col2) references tbl_name(col1,col2) on update RESTRICT)";
+        SqlParser parser = SqlParser.create(sql, PARSER_CONFIG);
+        try {
+            SqlNode sqlNode = parser.parseStmt();
+            assert sqlNode instanceof SqlCreateTable;
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    @Test
+    public void sqlDropConstraintForeign() {
+        String sql = "alter table t1 drop foreign key ke";
+        SqlParser parser = SqlParser.create(sql, PARSER_CONFIG);
+        try {
+            SqlNode sqlNode = parser.parseStmt();
+            assert sqlNode instanceof SqlAlterTable;
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
+
 }
diff --git a/dingo-executor/src/main/java/io/dingodb/server/executor/ddl/DdlServer.java b/dingo-executor/src/main/java/io/dingodb/server/executor/ddl/DdlServer.java
@@ -257,7 +257,6 @@ public static void delivery2worker(DdlWorker worker, DdlJob ddlJob, DdlWorkerPoo
                         try {
                             Pair<Boolean, Long> res = checkMDLInfo(ddlJob.getId());
                             if (res.getKey()) {
-                                pool.returnObject(worker);
                                 String error = DdlWorker.waitSchemaSyncedForMDL(dc, ddlJob, res.getValue());
                                 if (error != null) {
                                     LogUtils.warn(log, "[ddl] check MDL info failed, jobId:{}", ddlJob.getId());
@@ -268,15 +267,13 @@ public static void delivery2worker(DdlWorker worker, DdlJob ddlJob, DdlWorkerPoo
                                 return;
                             }
                         } catch (Exception e) {
-                            pool.returnObject(worker);
                             LogUtils.warn(log, "[ddl] check MDL info failed, jobId:{}", ddlJob.getId());
                             return;
                         }
                     } else {
                         try {
                             waitSchemaSynced(dc, ddlJob, 2 * dc.getLease(), worker);
                         } catch (Exception e) {
-                            pool.returnObject(worker);
                             LogUtils.error(log, "[ddl] wait ddl job sync failed, reason:" + e.getMessage()
                                 + ", job:" + ddlJob);
                             Utils.sleep(1000);

diff --git a/dingo-store-proxy/src/main/java/io/dingodb/store/proxy/ddl/DdlHandler.java b/dingo-store-proxy/src/main/java/io/dingodb/store/proxy/ddl/DdlHandler.java
@@ -397,10 +397,8 @@ public void addColumn(SchemaInfo schemaInfo, Table table, ColumnDefinition colum
     }
 
     public void dropColumn(
-        long schemaId,
-        String schemaName,
-        Long tableId,
-        String tableName,
+        SchemaInfo schemaInfo,
+        Table table,
         String columnName,
         String markDel, String relatedIndex,
         String connId
@@ -410,10 +408,10 @@ public void dropColumn(
         args.add(markDel);
         args.add(relatedIndex);
         DdlJob job = DdlJob.builder()
-            .schemaId(schemaId)
-            .tableId(tableId)
-            .schemaName(schemaName)
-            .tableName(tableName)
+            .schemaId(schemaInfo.getSchemaId())
+            .tableId(table.tableId.seq)
+            .schemaName(schemaInfo.getName())
+            .tableName(table.getName())
             .schemaState(SchemaState.SCHEMA_PUBLIC)
             .actionType(ActionType.ActionDropColumn)
             .build();
@@ -422,7 +420,7 @@ public void dropColumn(
             doDdlJob(job);
         } catch (Exception e) {
             LogUtils.error(log, "[ddl-error] dropColumn error, tableName:{}, columnName:{}",
-                tableName, columnName, e);
+                table.getName(), columnName, e);
             throw e;
         }
     }