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/10/14 09:49:50 UTC
[GitHub] [shardingsphere] jingshanglu opened a new issue #7791: Improve mysql Statemen definition and Visitor
jingshanglu opened a new issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791
## Background
Now, the Statement definition of mysql is not perfect yet. We need to refine the Statement definition and construct reasonable statements through visitor.
For example, some statements are empty
```
// shardingsphere/shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dml/MySQLCallStatement.java
package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dml;
/**
* MySQL call statement.
*/
public final class MySQLCallStatement extends CallStatement implements MySQLStatement {
}
// shardingsphere/shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/impl/MySQLDMLVisitor.java
package org.apache.shardingsphere.sql.parser.mysql.visitor.impl
/**
* DML visitor for MySQL.
*/
public final class MySQLDMLVisitor extends MySQLVisitor implements DMLVisitor {
@Override
public ASTNode visitCall(final CallContext ctx) {
return new MySQLCallStatement();
}
@Override
public ASTNode visitDoStatement(final DoStatementContext ctx) {
return new MySQLDoStatement();
}
}
```
## Tasks
1. Refine statement definition.
2. Implementation corresponding method in Visitor to builds a reasonable Statement.
## Solutions
1. Think about a reasonable Statement definition
2. Complete the specify Statement Class
3. Implementation corresponding method in Visitor(eg:shardingsphere/shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/impl/MySQLDMLVisitor.java)
4. Add unit test for it.
## Note
We can discuss the reasonableness of the Statement definition and then implement 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
[GitHub] [shardingsphere] wenweibin commented on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
wenweibin commented on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-726833383
Firstlly, I'm going to start with `MySQLDropTableStatement` of subtask 4.According the synax of dropTable in DDLStatement.g4, I think the refactor is like below：
1. Revise `MySQLDropTableStatement` class
```
/**
* MySQL drop table statement.
*/
@ToString
@Getter
@Setter
public final class MySQLDropTableStatement extends DropTableStatement implements MySQLStatement {
private boolean temporary;
private boolean isExisted;
private Restrict restrict;
}
```
2. Add `Restrict` enum
```
/**
* Restrict type enum.
*/
public enum Restrict {
RESTRICT, CASCADE
}
```
3. Revise `MySQLDDLStatementSQLVisitor` visitDropTable method
```
@Override
public ASTNode visitDropTable(final DropTableContext ctx) {
MySQLDropTableStatement result = new MySQLDropTableStatement();
result.setTemporary(Objects.nonNull(ctx.TEMPORARY()));
result.setExisted(Objects.nonNull(ctx.existClause()));
if (null != ctx.restrict()) {
result.setRestrict(Objects.nonNull(ctx.restrict()) ? Restrict.RESTRICT : Restrict.CASCADE);
}
result.getTables().addAll(((CollectionValue<SimpleTableSegment>) visit(ctx.tableNames())).getValue());
return result;
}
````
Can you give some suggestions？@jingshanglu
----------------------------------------------------------------
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
[GitHub] [shardingsphere] jingshanglu commented on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-748815710
@wenweibin I agree with your design.
----------------------------------------------------------------
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
[GitHub] [shardingsphere] jingshanglu commented on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-711887055
> Please assgin to me, i want to have a try
@wenweibin OK, we can discuss it first and then implement it to avoid unnecessary work.
----------------------------------------------------------------
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
[GitHub] [shardingsphere] jingshanglu commented on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-711889575
@wenweibin We can start with the example above(`MySQLCallStatement`,`MySQLDoStatement`) or the statement you are interested in.
----------------------------------------------------------------
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
[GitHub] [shardingsphere] wenweibin commented on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
wenweibin commented on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-715922626
Which project should unit tests be placed? `shardingsphere-sql-parser-statement` or `shardingsphere-sql-parser-mysql`？
If they are placed in the `shardingsphere-sql-parser-statement` directory, should these refactored codes migrate to `CallStatement` and `DoStatement`？
Hope to get your advices. @jingshanglu
----------------------------------------------------------------
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
[GitHub] [shardingsphere] jingshanglu commented on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-714185159
@wenweibin I think your design is ok.
----------------------------------------------------------------
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
[GitHub] [shardingsphere] jingshanglu commented on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-723735808
@wenweibin Hello, i add some subtask about `table`,we can discuss the definition first.
----------------------------------------------------------------
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
[GitHub] [shardingsphere] wenweibin edited a comment on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
wenweibin edited a comment on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-715922626
Which project should unit tests be placed? `shardingsphere-sql-parser-test` or `shardingsphere-sql-parser-mysql`？
If they are placed in the `shardingsphere-sql-parser-test` directory, should these refactored codes migrate to `CallStatement` and `DoStatement`？
Hope to get your advices. @jingshanglu
----------------------------------------------------------------
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
[GitHub] [shardingsphere] jingshanglu closed issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
jingshanglu closed issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791
----------------------------------------------------------------
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
[GitHub] [shardingsphere] jingshanglu commented on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-716342564
> Which project should unit tests be placed? `shardingsphere-sql-parser-test` or `shardingsphere-sql-parser-mysql`？
> If they are placed in the `shardingsphere-sql-parser-test` directory, should these refactored codes migrate to `CallStatement` and `DoStatement`？
> Hope to get your advices. @jingshanglu
For the logic of the test framework, please refer to #7880 . Test sql and expected result should place in `shardingsphere/shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/main/resources/`, and then add the assert code on project `shardingsphere-sql-parser-test`. @wenweibin
----------------------------------------------------------------
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
[GitHub] [shardingsphere] wenweibin commented on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
wenweibin commented on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-713653232
1. Considering that the synax of _MySQL_ CALL _Statement_ is `CALL sp_name([parameter[,...]])`,in my opinion `MySQLCallStatement` class should add two properties: `procedureName` and `parameters`,the codes are as below:
```
@ToString
@AllArgsConstructor
@Getter
public final class MySQLCallStatement extends CallStatement implements MySQLStatement {
private String procedureName;
private List<ExpressionSegment> parameters;
}
```
2. The Synax of _MySQL_ DO_Statement_ is `DO expr [, expr] ...,so i think `MySQLDoStatement` class should add one propertie: `expressionSegments` ,the codes are as below:
```
@ToString
@AllArgsConstructor
@Getter
public final class MySQLDoStatement extends DoStatement implements MySQLStatement {
private List<ExpressionSegment> expressionSegments;
}
```
3. Correspondingly，the `MySQLDMLVisitor` class is refactored as below：
```
public final class MySQLDMLVisitor extends MySQLVisitor implements DMLVisitor {
@Override
public ASTNode visitCall(final CallContext ctx) {
String procedureName = ctx.identifier().getText();
List<ExpressionSegment> parameters = new ArrayList<>();
ctx.expr().forEach(each -> parameters.add((ExpressionSegment) visit(each)));
return new MySQLCallStatement(procedureName,parameters);
}
@Override
public ASTNode visitDoStatement(final DoStatementContext ctx) {
List<ExpressionSegment> expressions = new ArrayList<>();
ctx.expr().forEach(each -> expressions.add((ExpressionSegment) visit(each)));
return new MySQLDoStatement(expressions);
}
}
```
Can you give some suggestions？@jingshanglu
----------------------------------------------------------------
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
[GitHub] [shardingsphere] wenweibin commented on issue #7791: Improve mysql Statemen definition and Visitor
Posted by GitBox <gi...@apache.org>.
wenweibin commented on issue #7791:
URL: https://github.com/apache/shardingsphere/issues/7791#issuecomment-709995803
Please assgin to me, i want to have a try
----------------------------------------------------------------
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