You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/01/31 07:07:10 UTC

[GitHub] [incubator-shardingsphere] SteNicholas opened a new pull request #4123: Add MySQL DDLStatement antlr visitor

SteNicholas opened a new pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123
 
 
   Fixes #3914.
   
   `MySQLVisitor` lacks of visitor for `DDLStatement.g4`. It is necessary to add `DDLStatement.g4` antlr visitor.
   
   Changes proposed in this pull request:
   - `MySQLVisitor` adds `DDLStatement.g4` antlr visitor.
   

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] SteNicholas commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373771791
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -190,6 +227,135 @@ public ASTNode visitCreateUser(final CreateUserContext ctx) {
     }
 
     // DDLStatement.g4
+    @Override
+    public ASTNode visitCreateTable(final CreateTableContext ctx) {
+        CreateTableStatement result = new CreateTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        CreateDefinitionClause_Context createDefinitionClause = ctx.createDefinitionClause_();
+        if (null != createDefinitionClause) {
+            for (CreateDefinition_Context createDefinition : createDefinitionClause.createDefinitions_().createDefinition_()) {
+                ColumnDefinitionContext columnDefinition = createDefinition.columnDefinition();
+                if (null != columnDefinition) {
+                    ColumnDefinitionSegment columnDefinitionSegment = createColumnDefinitionSegment(columnDefinition, result);
+                    result.getColumnDefinitions().add(columnDefinitionSegment);
+                    result.getAllSQLSegments().add(columnDefinitionSegment);
+                }
+                ConstraintDefinition_Context constraintDefinition = createDefinition.constraintDefinition_();
+                ForeignKeyOption_Context foreignKeyOption = null == constraintDefinition ? null : constraintDefinition.foreignKeyOption_();
+                if (null != foreignKeyOption) {
+                    result.getAllSQLSegments().add((TableSegment) visit(foreignKeyOption.referenceDefinition_().tableName()));
+                }
+            }
+        }
+        CreateLikeClause_Context createLikeClause = ctx.createLikeClause_();
+        if (null != createLikeClause) {
+            result.getAllSQLSegments().add((TableSegment) visit(createLikeClause));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitAlterTable(final AlterTableContext ctx) {
+        AlterTableStatement result = new AlterTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        if (null != ctx.alterDefinitionClause_()) {
+            for (AlterSpecification_Context alterSpecification : ctx.alterDefinitionClause_().alterSpecification_()) {
+                AddColumnSpecificationContext addColumnSpecification = alterSpecification.addColumnSpecification();
+                if (null != addColumnSpecification) {
+                    List<ColumnDefinitionContext> columnDefinitions = addColumnSpecification.columnDefinition();
+                    ColumnDefinitionSegment columnDefinitionSegment = null;
+                    for (ColumnDefinitionContext columnDefinition : columnDefinitions) {
+                        columnDefinitionSegment = createColumnDefinitionSegment(columnDefinition, result);
+                        result.getAddedColumnDefinitions().add(columnDefinitionSegment);
+                        result.getAllSQLSegments().add(columnDefinitionSegment);
+                    }
+                    createColumnPositionSegment(addColumnSpecification.firstOrAfterColumn(), columnDefinitionSegment, result);
+                }
+                AddConstraintSpecificationContext addConstraintSpecification = alterSpecification.addConstraintSpecification();
+                ForeignKeyOption_Context foreignKeyOption = null == addConstraintSpecification
+                        ? null : addConstraintSpecification.constraintDefinition_().foreignKeyOption_();
+                if (null != foreignKeyOption) {
+                    result.getAllSQLSegments().add((TableSegment) visit(foreignKeyOption.referenceDefinition_().tableName()));
+                }
+                ChangeColumnSpecificationContext changeColumnSpecification = alterSpecification.changeColumnSpecification();
+                if (null != changeColumnSpecification) {
+                    createColumnPositionSegment(changeColumnSpecification.firstOrAfterColumn(),
+                            createColumnDefinitionSegment(changeColumnSpecification.columnDefinition(), result), result);
+                }
+                DropColumnSpecificationContext dropColumnSpecification = alterSpecification.dropColumnSpecification();
+                if (null != dropColumnSpecification) {
+                    result.getDroppedColumnNames().add(((ColumnSegment) visit(dropColumnSpecification)).getName());
+                }
+                ModifyColumnSpecificationContext modifyColumnSpecification = alterSpecification.modifyColumnSpecification();
+                if (null != modifyColumnSpecification) {
+                    createColumnPositionSegment(modifyColumnSpecification.firstOrAfterColumn(),
+                            createColumnDefinitionSegment(modifyColumnSpecification.columnDefinition(), result), result);
+                }
+            }
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitDropTable(final DropTableContext ctx) {
+        DropTableStatement result = new DropTableStatement();
+        ListValue<TableSegment> tables = (ListValue<TableSegment>) visit(ctx.tableNames());
+        result.getTables().addAll(tables.getValues());
+        result.getAllSQLSegments().addAll(tables.getValues());
+        return result;
+    }
+
+    @Override
+    public ASTNode visitTruncateTable(final TruncateTableContext ctx) {
+        DDLStatement result = new DDLStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.getAllSQLSegments().add(table);
+        return result;
+    }
+
+    @Override
+    public ASTNode visitCreateIndex(final CreateIndexContext ctx) {
+        CreateIndexStatement result = new CreateIndexStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        IndexSegment index = (IndexSegment) visit(ctx.indexName());
+        result.setIndex(index);
+        result.getAllSQLSegments().add(index);
+        return result;
+    }
+
+    @Override
+    public ASTNode visitDropIndex(final DropIndexContext ctx) {
+        DropIndexStatement result = new DropIndexStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        IndexSegment index = (IndexSegment) visit(ctx.indexName());
 
 Review comment:
   @tristaZero Okay, I would like to remove `IndexSegment`.

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373757511
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -190,6 +227,135 @@ public ASTNode visitCreateUser(final CreateUserContext ctx) {
     }
 
     // DDLStatement.g4
+    @Override
+    public ASTNode visitCreateTable(final CreateTableContext ctx) {
+        CreateTableStatement result = new CreateTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        CreateDefinitionClause_Context createDefinitionClause = ctx.createDefinitionClause_();
+        if (null != createDefinitionClause) {
 
 Review comment:
   IMO, it is better to put it to visitCreateDefinitionClause(). Parent visior has no reposibility to visit its child visitor, and just gets the result returned by its child.

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373778942
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -871,6 +1051,53 @@ private ASTNode createExpressionSegment(final ASTNode astNode, final ParserRuleC
         return astNode;
     }
     
+    private ColumnDefinitionSegment createColumnDefinitionSegment(final ColumnDefinitionContext columnDefinition, final DDLStatement statement) {
+        ColumnSegment column = (ColumnSegment) visit(columnDefinition.columnName());
+        LiteralValue dataType = (LiteralValue) visit(columnDefinition.dataType().dataTypeName_());
+        boolean isPrimaryKey = false;
+        for (InlineDataType_Context inlineDataType : columnDefinition.inlineDataType_()) {
+            CommonDataTypeOption_Context commonDataTypeOption = inlineDataType.commonDataTypeOption_();
+            if (null != commonDataTypeOption) {
+                if (null != commonDataTypeOption.primaryKey()) {
+                    isPrimaryKey = true;
+                }
+                if (null != commonDataTypeOption.referenceDefinition_()) {
+                    statement.getAllSQLSegments().add((TableSegment) visit(commonDataTypeOption.referenceDefinition_().tableName()));
+                }
+            }
+        }
+        for (GeneratedDataType_Context generatedDataType: columnDefinition.generatedDataType_()) {
+            CommonDataTypeOption_Context commonDataTypeOption = generatedDataType.commonDataTypeOption_();
+            if (null != commonDataTypeOption) {
+                if (null != commonDataTypeOption.primaryKey()) {
+                    isPrimaryKey = true;
+                }
+                if (null != commonDataTypeOption.referenceDefinition_()) {
+                    statement.getAllSQLSegments().add((TableSegment) visit(commonDataTypeOption.referenceDefinition_().tableName()));
 
 Review comment:
   To create `visitReferenceDefinition_()` is better. Short and simple function always make peple easy to read. Other than, one function is supposed to focus on itself, not include its child.

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373756694
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -190,6 +227,135 @@ public ASTNode visitCreateUser(final CreateUserContext ctx) {
     }
 
     // DDLStatement.g4
+    @Override
+    public ASTNode visitCreateTable(final CreateTableContext ctx) {
+        CreateTableStatement result = new CreateTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        CreateDefinitionClause_Context createDefinitionClause = ctx.createDefinitionClause_();
+        if (null != createDefinitionClause) {
+            for (CreateDefinition_Context createDefinition : createDefinitionClause.createDefinitions_().createDefinition_()) {
+                ColumnDefinitionContext columnDefinition = createDefinition.columnDefinition();
+                if (null != columnDefinition) {
+                    ColumnDefinitionSegment columnDefinitionSegment = createColumnDefinitionSegment(columnDefinition, result);
+                    result.getColumnDefinitions().add(columnDefinitionSegment);
+                    result.getAllSQLSegments().add(columnDefinitionSegment);
+                }
+                ConstraintDefinition_Context constraintDefinition = createDefinition.constraintDefinition_();
+                ForeignKeyOption_Context foreignKeyOption = null == constraintDefinition ? null : constraintDefinition.foreignKeyOption_();
+                if (null != foreignKeyOption) {
+                    result.getAllSQLSegments().add((TableSegment) visit(foreignKeyOption.referenceDefinition_().tableName()));
+                }
+            }
+        }
+        CreateLikeClause_Context createLikeClause = ctx.createLikeClause_();
+        if (null != createLikeClause) {
+            result.getAllSQLSegments().add((TableSegment) visit(createLikeClause));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitAlterTable(final AlterTableContext ctx) {
+        AlterTableStatement result = new AlterTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        if (null != ctx.alterDefinitionClause_()) {
+            for (AlterSpecification_Context alterSpecification : ctx.alterDefinitionClause_().alterSpecification_()) {
+                AddColumnSpecificationContext addColumnSpecification = alterSpecification.addColumnSpecification();
+                if (null != addColumnSpecification) {
+                    List<ColumnDefinitionContext> columnDefinitions = addColumnSpecification.columnDefinition();
+                    ColumnDefinitionSegment columnDefinitionSegment = null;
+                    for (ColumnDefinitionContext columnDefinition : columnDefinitions) {
+                        columnDefinitionSegment = createColumnDefinitionSegment(columnDefinition, result);
+                        result.getAddedColumnDefinitions().add(columnDefinitionSegment);
+                        result.getAllSQLSegments().add(columnDefinitionSegment);
+                    }
+                    createColumnPositionSegment(addColumnSpecification.firstOrAfterColumn(), columnDefinitionSegment, result);
+                }
+                AddConstraintSpecificationContext addConstraintSpecification = alterSpecification.addConstraintSpecification();
+                ForeignKeyOption_Context foreignKeyOption = null == addConstraintSpecification
+                        ? null : addConstraintSpecification.constraintDefinition_().foreignKeyOption_();
+                if (null != foreignKeyOption) {
+                    result.getAllSQLSegments().add((TableSegment) visit(foreignKeyOption.referenceDefinition_().tableName()));
+                }
+                ChangeColumnSpecificationContext changeColumnSpecification = alterSpecification.changeColumnSpecification();
+                if (null != changeColumnSpecification) {
+                    createColumnPositionSegment(changeColumnSpecification.firstOrAfterColumn(),
+                            createColumnDefinitionSegment(changeColumnSpecification.columnDefinition(), result), result);
+                }
+                DropColumnSpecificationContext dropColumnSpecification = alterSpecification.dropColumnSpecification();
+                if (null != dropColumnSpecification) {
+                    result.getDroppedColumnNames().add(((ColumnSegment) visit(dropColumnSpecification)).getName());
+                }
+                ModifyColumnSpecificationContext modifyColumnSpecification = alterSpecification.modifyColumnSpecification();
+                if (null != modifyColumnSpecification) {
+                    createColumnPositionSegment(modifyColumnSpecification.firstOrAfterColumn(),
+                            createColumnDefinitionSegment(modifyColumnSpecification.columnDefinition(), result), result);
+                }
+            }
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitDropTable(final DropTableContext ctx) {
+        DropTableStatement result = new DropTableStatement();
+        ListValue<TableSegment> tables = (ListValue<TableSegment>) visit(ctx.tableNames());
+        result.getTables().addAll(tables.getValues());
+        result.getAllSQLSegments().addAll(tables.getValues());
+        return result;
+    }
+
+    @Override
+    public ASTNode visitTruncateTable(final TruncateTableContext ctx) {
+        DDLStatement result = new DDLStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.getAllSQLSegments().add(table);
+        return result;
+    }
+
+    @Override
+    public ASTNode visitCreateIndex(final CreateIndexContext ctx) {
+        CreateIndexStatement result = new CreateIndexStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        IndexSegment index = (IndexSegment) visit(ctx.indexName());
+        result.setIndex(index);
+        result.getAllSQLSegments().add(index);
+        return result;
+    }
+
+    @Override
+    public ASTNode visitDropIndex(final DropIndexContext ctx) {
+        DropIndexStatement result = new DropIndexStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        IndexSegment index = (IndexSegment) visit(ctx.indexName());
 
 Review comment:
   META-INF/parsing-rule-definition/mysql/sql-statement-rule-definition.xml tells us there is no need to add IndexSegment to CreateIndexStatement when DB is MySQL.

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373778674
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -190,6 +227,135 @@ public ASTNode visitCreateUser(final CreateUserContext ctx) {
     }
 
     // DDLStatement.g4
+    @Override
+    public ASTNode visitCreateTable(final CreateTableContext ctx) {
+        CreateTableStatement result = new CreateTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        CreateDefinitionClause_Context createDefinitionClause = ctx.createDefinitionClause_();
+        if (null != createDefinitionClause) {
 
 Review comment:
   @SteNicholas Could get `ListValue<ColumnDefinitionSegment>` from `visitCreateDefinitionClause()` and add it to `columnDefinitions` in `CreateTableStatement`

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] SteNicholas commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373771777
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -190,6 +227,135 @@ public ASTNode visitCreateUser(final CreateUserContext ctx) {
     }
 
     // DDLStatement.g4
+    @Override
+    public ASTNode visitCreateTable(final CreateTableContext ctx) {
+        CreateTableStatement result = new CreateTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        CreateDefinitionClause_Context createDefinitionClause = ctx.createDefinitionClause_();
+        if (null != createDefinitionClause) {
+            for (CreateDefinition_Context createDefinition : createDefinitionClause.createDefinitions_().createDefinition_()) {
+                ColumnDefinitionContext columnDefinition = createDefinition.columnDefinition();
+                if (null != columnDefinition) {
+                    ColumnDefinitionSegment columnDefinitionSegment = createColumnDefinitionSegment(columnDefinition, result);
+                    result.getColumnDefinitions().add(columnDefinitionSegment);
+                    result.getAllSQLSegments().add(columnDefinitionSegment);
+                }
+                ConstraintDefinition_Context constraintDefinition = createDefinition.constraintDefinition_();
+                ForeignKeyOption_Context foreignKeyOption = null == constraintDefinition ? null : constraintDefinition.foreignKeyOption_();
+                if (null != foreignKeyOption) {
+                    result.getAllSQLSegments().add((TableSegment) visit(foreignKeyOption.referenceDefinition_().tableName()));
+                }
+            }
+        }
+        CreateLikeClause_Context createLikeClause = ctx.createLikeClause_();
+        if (null != createLikeClause) {
+            result.getAllSQLSegments().add((TableSegment) visit(createLikeClause));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitAlterTable(final AlterTableContext ctx) {
+        AlterTableStatement result = new AlterTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        if (null != ctx.alterDefinitionClause_()) {
+            for (AlterSpecification_Context alterSpecification : ctx.alterDefinitionClause_().alterSpecification_()) {
+                AddColumnSpecificationContext addColumnSpecification = alterSpecification.addColumnSpecification();
+                if (null != addColumnSpecification) {
+                    List<ColumnDefinitionContext> columnDefinitions = addColumnSpecification.columnDefinition();
+                    ColumnDefinitionSegment columnDefinitionSegment = null;
+                    for (ColumnDefinitionContext columnDefinition : columnDefinitions) {
+                        columnDefinitionSegment = createColumnDefinitionSegment(columnDefinition, result);
+                        result.getAddedColumnDefinitions().add(columnDefinitionSegment);
+                        result.getAllSQLSegments().add(columnDefinitionSegment);
+                    }
+                    createColumnPositionSegment(addColumnSpecification.firstOrAfterColumn(), columnDefinitionSegment, result);
+                }
+                AddConstraintSpecificationContext addConstraintSpecification = alterSpecification.addConstraintSpecification();
+                ForeignKeyOption_Context foreignKeyOption = null == addConstraintSpecification
+                        ? null : addConstraintSpecification.constraintDefinition_().foreignKeyOption_();
+                if (null != foreignKeyOption) {
+                    result.getAllSQLSegments().add((TableSegment) visit(foreignKeyOption.referenceDefinition_().tableName()));
+                }
+                ChangeColumnSpecificationContext changeColumnSpecification = alterSpecification.changeColumnSpecification();
+                if (null != changeColumnSpecification) {
+                    createColumnPositionSegment(changeColumnSpecification.firstOrAfterColumn(),
+                            createColumnDefinitionSegment(changeColumnSpecification.columnDefinition(), result), result);
+                }
+                DropColumnSpecificationContext dropColumnSpecification = alterSpecification.dropColumnSpecification();
+                if (null != dropColumnSpecification) {
+                    result.getDroppedColumnNames().add(((ColumnSegment) visit(dropColumnSpecification)).getName());
+                }
+                ModifyColumnSpecificationContext modifyColumnSpecification = alterSpecification.modifyColumnSpecification();
+                if (null != modifyColumnSpecification) {
+                    createColumnPositionSegment(modifyColumnSpecification.firstOrAfterColumn(),
+                            createColumnDefinitionSegment(modifyColumnSpecification.columnDefinition(), result), result);
+                }
+            }
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitDropTable(final DropTableContext ctx) {
+        DropTableStatement result = new DropTableStatement();
+        ListValue<TableSegment> tables = (ListValue<TableSegment>) visit(ctx.tableNames());
+        result.getTables().addAll(tables.getValues());
+        result.getAllSQLSegments().addAll(tables.getValues());
+        return result;
+    }
+
+    @Override
+    public ASTNode visitTruncateTable(final TruncateTableContext ctx) {
+        DDLStatement result = new DDLStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.getAllSQLSegments().add(table);
+        return result;
+    }
+
+    @Override
+    public ASTNode visitCreateIndex(final CreateIndexContext ctx) {
+        CreateIndexStatement result = new CreateIndexStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        IndexSegment index = (IndexSegment) visit(ctx.indexName());
 
 Review comment:
   @tristaZero Okay, I would like to remove `IndexSegment`.

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] SteNicholas commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373774326
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -190,6 +227,135 @@ public ASTNode visitCreateUser(final CreateUserContext ctx) {
     }
 
     // DDLStatement.g4
+    @Override
+    public ASTNode visitCreateTable(final CreateTableContext ctx) {
+        CreateTableStatement result = new CreateTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        CreateDefinitionClause_Context createDefinitionClause = ctx.createDefinitionClause_();
+        if (null != createDefinitionClause) {
 
 Review comment:
   @tristaZero No, there is no any statement match for visitCreateDefinitionClause() result.

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] tristaZero merged pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
tristaZero merged pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123
 
 
   

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373756688
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -190,6 +227,135 @@ public ASTNode visitCreateUser(final CreateUserContext ctx) {
     }
 
     // DDLStatement.g4
+    @Override
+    public ASTNode visitCreateTable(final CreateTableContext ctx) {
+        CreateTableStatement result = new CreateTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        CreateDefinitionClause_Context createDefinitionClause = ctx.createDefinitionClause_();
+        if (null != createDefinitionClause) {
+            for (CreateDefinition_Context createDefinition : createDefinitionClause.createDefinitions_().createDefinition_()) {
+                ColumnDefinitionContext columnDefinition = createDefinition.columnDefinition();
+                if (null != columnDefinition) {
+                    ColumnDefinitionSegment columnDefinitionSegment = createColumnDefinitionSegment(columnDefinition, result);
+                    result.getColumnDefinitions().add(columnDefinitionSegment);
+                    result.getAllSQLSegments().add(columnDefinitionSegment);
+                }
+                ConstraintDefinition_Context constraintDefinition = createDefinition.constraintDefinition_();
+                ForeignKeyOption_Context foreignKeyOption = null == constraintDefinition ? null : constraintDefinition.foreignKeyOption_();
+                if (null != foreignKeyOption) {
+                    result.getAllSQLSegments().add((TableSegment) visit(foreignKeyOption.referenceDefinition_().tableName()));
+                }
+            }
+        }
+        CreateLikeClause_Context createLikeClause = ctx.createLikeClause_();
+        if (null != createLikeClause) {
+            result.getAllSQLSegments().add((TableSegment) visit(createLikeClause));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitAlterTable(final AlterTableContext ctx) {
+        AlterTableStatement result = new AlterTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        if (null != ctx.alterDefinitionClause_()) {
+            for (AlterSpecification_Context alterSpecification : ctx.alterDefinitionClause_().alterSpecification_()) {
+                AddColumnSpecificationContext addColumnSpecification = alterSpecification.addColumnSpecification();
+                if (null != addColumnSpecification) {
+                    List<ColumnDefinitionContext> columnDefinitions = addColumnSpecification.columnDefinition();
+                    ColumnDefinitionSegment columnDefinitionSegment = null;
+                    for (ColumnDefinitionContext columnDefinition : columnDefinitions) {
+                        columnDefinitionSegment = createColumnDefinitionSegment(columnDefinition, result);
+                        result.getAddedColumnDefinitions().add(columnDefinitionSegment);
+                        result.getAllSQLSegments().add(columnDefinitionSegment);
+                    }
+                    createColumnPositionSegment(addColumnSpecification.firstOrAfterColumn(), columnDefinitionSegment, result);
+                }
+                AddConstraintSpecificationContext addConstraintSpecification = alterSpecification.addConstraintSpecification();
+                ForeignKeyOption_Context foreignKeyOption = null == addConstraintSpecification
+                        ? null : addConstraintSpecification.constraintDefinition_().foreignKeyOption_();
+                if (null != foreignKeyOption) {
+                    result.getAllSQLSegments().add((TableSegment) visit(foreignKeyOption.referenceDefinition_().tableName()));
+                }
+                ChangeColumnSpecificationContext changeColumnSpecification = alterSpecification.changeColumnSpecification();
+                if (null != changeColumnSpecification) {
+                    createColumnPositionSegment(changeColumnSpecification.firstOrAfterColumn(),
+                            createColumnDefinitionSegment(changeColumnSpecification.columnDefinition(), result), result);
+                }
+                DropColumnSpecificationContext dropColumnSpecification = alterSpecification.dropColumnSpecification();
+                if (null != dropColumnSpecification) {
+                    result.getDroppedColumnNames().add(((ColumnSegment) visit(dropColumnSpecification)).getName());
+                }
+                ModifyColumnSpecificationContext modifyColumnSpecification = alterSpecification.modifyColumnSpecification();
+                if (null != modifyColumnSpecification) {
+                    createColumnPositionSegment(modifyColumnSpecification.firstOrAfterColumn(),
+                            createColumnDefinitionSegment(modifyColumnSpecification.columnDefinition(), result), result);
+                }
+            }
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitDropTable(final DropTableContext ctx) {
+        DropTableStatement result = new DropTableStatement();
+        ListValue<TableSegment> tables = (ListValue<TableSegment>) visit(ctx.tableNames());
+        result.getTables().addAll(tables.getValues());
+        result.getAllSQLSegments().addAll(tables.getValues());
+        return result;
+    }
+
+    @Override
+    public ASTNode visitTruncateTable(final TruncateTableContext ctx) {
+        DDLStatement result = new DDLStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.getAllSQLSegments().add(table);
+        return result;
+    }
+
+    @Override
+    public ASTNode visitCreateIndex(final CreateIndexContext ctx) {
+        CreateIndexStatement result = new CreateIndexStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        IndexSegment index = (IndexSegment) visit(ctx.indexName());
 
 Review comment:
   `META-INF/parsing-rule-definition/mysql/sql-statement-rule-definition.xml` tells us there is no need to add IndexSegment to CreateIndexStatement when DB is MySQL. 

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373757785
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -190,6 +227,135 @@ public ASTNode visitCreateUser(final CreateUserContext ctx) {
     }
 
     // DDLStatement.g4
+    @Override
+    public ASTNode visitCreateTable(final CreateTableContext ctx) {
+        CreateTableStatement result = new CreateTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        CreateDefinitionClause_Context createDefinitionClause = ctx.createDefinitionClause_();
+        if (null != createDefinitionClause) {
+            for (CreateDefinition_Context createDefinition : createDefinitionClause.createDefinitions_().createDefinition_()) {
+                ColumnDefinitionContext columnDefinition = createDefinition.columnDefinition();
+                if (null != columnDefinition) {
+                    ColumnDefinitionSegment columnDefinitionSegment = createColumnDefinitionSegment(columnDefinition, result);
+                    result.getColumnDefinitions().add(columnDefinitionSegment);
+                    result.getAllSQLSegments().add(columnDefinitionSegment);
+                }
+                ConstraintDefinition_Context constraintDefinition = createDefinition.constraintDefinition_();
+                ForeignKeyOption_Context foreignKeyOption = null == constraintDefinition ? null : constraintDefinition.foreignKeyOption_();
+                if (null != foreignKeyOption) {
+                    result.getAllSQLSegments().add((TableSegment) visit(foreignKeyOption.referenceDefinition_().tableName()));
+                }
+            }
+        }
+        CreateLikeClause_Context createLikeClause = ctx.createLikeClause_();
+        if (null != createLikeClause) {
+            result.getAllSQLSegments().add((TableSegment) visit(createLikeClause));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitAlterTable(final AlterTableContext ctx) {
+        AlterTableStatement result = new AlterTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        if (null != ctx.alterDefinitionClause_()) {
 
 Review comment:
   The above suggestion applied to the following visitors. Could you review those rather long visitors for optimizing?

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373778970
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -871,6 +1051,53 @@ private ASTNode createExpressionSegment(final ASTNode astNode, final ParserRuleC
         return astNode;
     }
     
+    private ColumnDefinitionSegment createColumnDefinitionSegment(final ColumnDefinitionContext columnDefinition, final DDLStatement statement) {
+        ColumnSegment column = (ColumnSegment) visit(columnDefinition.columnName());
+        LiteralValue dataType = (LiteralValue) visit(columnDefinition.dataType().dataTypeName_());
+        boolean isPrimaryKey = false;
+        for (InlineDataType_Context inlineDataType : columnDefinition.inlineDataType_()) {
+            CommonDataTypeOption_Context commonDataTypeOption = inlineDataType.commonDataTypeOption_();
+            if (null != commonDataTypeOption) {
+                if (null != commonDataTypeOption.primaryKey()) {
+                    isPrimaryKey = true;
+                }
+                if (null != commonDataTypeOption.referenceDefinition_()) {
+                    statement.getAllSQLSegments().add((TableSegment) visit(commonDataTypeOption.referenceDefinition_().tableName()));
+                }
+            }
+        }
+        for (GeneratedDataType_Context generatedDataType: columnDefinition.generatedDataType_()) {
+            CommonDataTypeOption_Context commonDataTypeOption = generatedDataType.commonDataTypeOption_();
+            if (null != commonDataTypeOption) {
+                if (null != commonDataTypeOption.primaryKey()) {
+                    isPrimaryKey = true;
+                }
+                if (null != commonDataTypeOption.referenceDefinition_()) {
+                    statement.getAllSQLSegments().add((TableSegment) visit(commonDataTypeOption.referenceDefinition_().tableName()));
 
 Review comment:
   Could give more review on other long funcions?

----------------------------------------------------------------
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

[GitHub] [incubator-shardingsphere] SteNicholas commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor

Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #4123: Add MySQL DDLStatement antlr visitor
URL: https://github.com/apache/incubator-shardingsphere/pull/4123#discussion_r373774407
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/MySQLVisitor.java
 ##########
 @@ -190,6 +227,135 @@ public ASTNode visitCreateUser(final CreateUserContext ctx) {
     }
 
     // DDLStatement.g4
+    @Override
+    public ASTNode visitCreateTable(final CreateTableContext ctx) {
+        CreateTableStatement result = new CreateTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        CreateDefinitionClause_Context createDefinitionClause = ctx.createDefinitionClause_();
+        if (null != createDefinitionClause) {
+            for (CreateDefinition_Context createDefinition : createDefinitionClause.createDefinitions_().createDefinition_()) {
+                ColumnDefinitionContext columnDefinition = createDefinition.columnDefinition();
+                if (null != columnDefinition) {
+                    ColumnDefinitionSegment columnDefinitionSegment = createColumnDefinitionSegment(columnDefinition, result);
+                    result.getColumnDefinitions().add(columnDefinitionSegment);
+                    result.getAllSQLSegments().add(columnDefinitionSegment);
+                }
+                ConstraintDefinition_Context constraintDefinition = createDefinition.constraintDefinition_();
+                ForeignKeyOption_Context foreignKeyOption = null == constraintDefinition ? null : constraintDefinition.foreignKeyOption_();
+                if (null != foreignKeyOption) {
+                    result.getAllSQLSegments().add((TableSegment) visit(foreignKeyOption.referenceDefinition_().tableName()));
+                }
+            }
+        }
+        CreateLikeClause_Context createLikeClause = ctx.createLikeClause_();
+        if (null != createLikeClause) {
+            result.getAllSQLSegments().add((TableSegment) visit(createLikeClause));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitAlterTable(final AlterTableContext ctx) {
+        AlterTableStatement result = new AlterTableStatement();
+        TableSegment table = (TableSegment) visit(ctx.tableName());
+        result.setTable(table);
+        result.getAllSQLSegments().add(table);
+        if (null != ctx.alterDefinitionClause_()) {
 
 Review comment:
   @tristaZero I knew your meaning. I have already previously optimized for the visitor, but no any statement contains `TableSegment`, cause that no match for visit**().

----------------------------------------------------------------
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