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/04/20 10:40:42 UTC
[GitHub] [shardingsphere] jingshanglu opened a new pull request #5241: fix for test of subquerytable
jingshanglu opened a new pull request #5241:
URL: https://github.com/apache/shardingsphere/pull/5241
Ref #4887.
Changes proposed in this pull request:
- fix for test of subquerytable
----------------------------------------------------------------
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] tristaZero commented on a change in pull request #5241: fix for test of subquerytable
Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5241:
URL: https://github.com/apache/shardingsphere/pull/5241#discussion_r411891399
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/java/org/apache/shardingsphere/sql/parser/oracle/visitor/impl/OracleDMLVisitor.java
##########
@@ -474,4 +474,9 @@ public ASTNode visitGroupByClause(final GroupByClauseContext ctx) {
}
return new GroupBySegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), items);
}
+
+ @Override
+ public ASTNode visitSubquery(final OracleStatementParser.SubqueryContext ctx) {
Review comment:
Is it possible to use `SubqueryContext` instead of `OracleStatementParser.SubqueryContext`
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sqlserver/src/main/java/org/apache/shardingsphere/sql/parser/sqlserver/visitor/impl/SQLServerDMLVisitor.java
##########
@@ -469,4 +469,9 @@ public ASTNode visitGroupByClause(final GroupByClauseContext ctx) {
}
return new GroupBySegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), items);
}
+
+ @Override
+ public ASTNode visitSubquery(final SQLServerStatementParser.SubqueryContext ctx) {
+ return visit(ctx.unionClause());
+ }
Review comment:
Do you think `SQL92DMLVisitor` need this function as well?
----------------------------------------------------------------
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 a change in pull request #5241: fix for test of subquerytable
Posted by GitBox <gi...@apache.org>.
jingshanglu commented on a change in pull request #5241:
URL: https://github.com/apache/shardingsphere/pull/5241#discussion_r411894392
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/java/org/apache/shardingsphere/sql/parser/oracle/visitor/impl/OracleDMLVisitor.java
##########
@@ -474,4 +474,9 @@ public ASTNode visitGroupByClause(final GroupByClauseContext ctx) {
}
return new GroupBySegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), items);
}
+
+ @Override
+ public ASTNode visitSubquery(final OracleStatementParser.SubqueryContext ctx) {
Review comment:
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] tristaZero commented on a change in pull request #5241: fix for test of subquerytable
Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5241:
URL: https://github.com/apache/shardingsphere/pull/5241#discussion_r411973107
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sql92/src/main/java/org/apache/shardingsphere/sql/parser/sql92/visitor/impl/SQL92DMLVisitor.java
##########
@@ -413,7 +414,7 @@ public ASTNode visitJoinSpecification(final JoinSpecificationContext ctx) {
}
if (null != ctx.USING()) {
Collection<ColumnSegment> columnSegmentList = new LinkedList<>();
- for (SQL92StatementParser.ColumnNameContext cname :ctx.columnNames().columnName()) {
+ for (ColumnNameContext cname :ctx.columnNames().columnName()) {
Review comment:
Please check style.
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/java/org/apache/shardingsphere/sql/parser/oracle/visitor/impl/OracleDMLVisitor.java
##########
@@ -444,7 +445,7 @@ public ASTNode visitJoinSpecification(final JoinSpecificationContext ctx) {
}
if (null != ctx.USING()) {
Collection<ColumnSegment> columnSegmentList = new LinkedList<>();
- for (OracleStatementParser.ColumnNameContext cname :ctx.columnNames().columnName()) {
+ for (ColumnNameContext cname :ctx.columnNames().columnName()) {
Review comment:
`ColumnNameContext cname :ctx.columnNames().columnName()`
`cname` ---> `each`
A blank space is needed in front of `ctx.columnNames().columnName()`
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sqlserver/src/main/java/org/apache/shardingsphere/sql/parser/sqlserver/visitor/impl/SQLServerDMLVisitor.java
##########
@@ -469,4 +469,9 @@ public ASTNode visitGroupByClause(final GroupByClauseContext ctx) {
}
return new GroupBySegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), items);
}
+
+ @Override
+ public ASTNode visitSubquery(final SQLServerStatementParser.SubqueryContext ctx) {
Review comment:
`final SQLServerStatementParser.SubqueryContext ctx`
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/postgresql/visitor/impl/PostgreSQLDMLVisitor.java
##########
@@ -529,4 +529,9 @@ public ASTNode visitLimitOffset(final LimitOffsetContext ctx) {
}
return new ParameterMarkerLimitValueSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), ((ParameterMarkerValue) visit(ctx.parameterMarker())).getValue());
}
+
+ @Override
+ public ASTNode visitSubquery(final PostgreSQLStatementParser.SubqueryContext ctx) {
Review comment:
`final PostgreSQLStatementParser.SubqueryContext ctx` --->`final SubqueryContext ctx`
----------------------------------------------------------------
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 a change in pull request #5241: fix for test of subquerytable
Posted by GitBox <gi...@apache.org>.
jingshanglu commented on a change in pull request #5241:
URL: https://github.com/apache/shardingsphere/pull/5241#discussion_r411894456
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sqlserver/src/main/java/org/apache/shardingsphere/sql/parser/sqlserver/visitor/impl/SQLServerDMLVisitor.java
##########
@@ -469,4 +469,9 @@ public ASTNode visitGroupByClause(final GroupByClauseContext ctx) {
}
return new GroupBySegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), items);
}
+
+ @Override
+ public ASTNode visitSubquery(final SQLServerStatementParser.SubqueryContext ctx) {
+ return visit(ctx.unionClause());
+ }
Review comment:
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