[GitHub] [shardingsphere] tristaZero commented on a change in pull request #6319: fix for rewrite of subquery

[GitBox <gi...@apache.org>]

tristaZero commented on a change in pull request #6319:
URL: https://github.com/apache/shardingsphere/pull/6319#discussion_r453454647



##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-rewrite/src/test/resources/sharding/select.xml
##########
@@ -28,8 +28,8 @@
     </rewrite-assertion>
 
     <rewrite-assertion id="select_with_subquery" db-type="MySQL">
-        <input sql="SELECT * FROM (select * from t_account where account_id=?) a WHERE account_id = 100" parameters="100" />
-        <output sql="SELECT * FROM (select * from t_account_0 where account_id=?) a WHERE account_id = 100" parameters="100" />
+        <input sql="SELECT * FROM (select t_account.account_id from t_account where t_account.account_id=?) a WHERE account_id = 100" parameters="100" />

Review comment:
       Are there other cases covering more the subqueries?

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/statement/dml/SelectStatementContext.java
##########
@@ -192,42 +193,32 @@ public boolean isSameGroupByAndOrderByItems() {
     
     @Override
     public Collection<SimpleTableSegment> getAllTables() {
-        Collection<SimpleTableSegment> result = new LinkedList<>(getSimpleTableSegments());
-        if (getSqlStatement().getWhere().isPresent()) {
-            result.addAll(getAllTablesFromWhere(getSqlStatement().getWhere().get()));
-        }
-        result.addAll(getAllTablesFromProjections(getSqlStatement().getProjections()));
-        if (getSqlStatement().getGroupBy().isPresent()) {
-            result.addAll(getAllTablesFromOrderByItems(getSqlStatement().getGroupBy().get().getGroupByItems()));
-        }
-        if (getSqlStatement().getOrderBy().isPresent()) {
-            result.addAll(getAllTablesFromOrderByItems(getSqlStatement().getOrderBy().get().getOrderByItems()));
-        }
+        Collection<SimpleTableSegment> result = getTableFromSelect(getSqlStatement());
         return result;

Review comment:
       It seems two of those statements can be merged into one. Please simplify 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