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/16 06:20:00 UTC

[GitHub] [incubator-shardingsphere] jingshanglu opened a new pull request #5206: Simplify the SelectStatement

jingshanglu opened a new pull request #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206
 
 
   Fixes #5135.
   
   Changes proposed in this pull request:
   - Simplify the SelectStatement
   

----------------------------------------------------------------
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 #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#discussion_r410049256
 
 

 ##########
 File path: encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
 ##########
 @@ -51,14 +53,15 @@
     
     @Override
     protected boolean isGenerateSQLTokenForEncrypt(final SQLStatementContext sqlStatementContext) {
-        return sqlStatementContext instanceof SelectStatementContext && !((SelectStatementContext) sqlStatementContext).getSqlStatement().getSimpleTableSegments().isEmpty();
+        SQLStatement sqlStatement = sqlStatementContext.getSqlStatement();
+        return sqlStatementContext instanceof SelectStatementContext && !((SelectStatementContext) sqlStatementContext).getSimpleTableSegments((SelectStatement) sqlStatement).isEmpty();
 
 Review comment:
   1. `getSimpleTableSegments((SelectStatement) sqlStatement)` 

----------------------------------------------------------------
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 #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#discussion_r410049702
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/statement/dml/SelectStatementContext.java
 ##########
 @@ -262,8 +268,86 @@ private boolean isTable(final OwnerSegment owner, final Collection<SimpleTableSe
         return true;
     }
     
+    private boolean isTable(final SimpleTableSegment owner, final Collection<SimpleTableSegment> tableSegments) {
+        for (SimpleTableSegment each : tableSegments) {
+            String tableName = owner.getTableName().getIdentifier().getValue();
+            if (tableName.equals(each.getAlias().orElse(null)) && !tableName.equals(each.getTableName().getIdentifier().getValue())) {
+                return false;
+            }
+        }
+        return true;
+    }
+    
     @Override
     public Optional<WhereSegment> getWhere() {
         return getSqlStatement().getWhere();
     }
+    
+    /**
+     * get tables.
+     *
+     * @param selectStatement SelectStatement.
+     *
+     * @return tables.
+     */
+    public Collection<SimpleTableSegment> getSimpleTableSegments(final SelectStatement selectStatement) {
+        Collection<SimpleTableSegment> result = getTables(selectStatement);
+        Collection<SimpleTableSegment> tables = new LinkedList<>();
+        for (SimpleTableSegment each : result) {
+            if (isTable(each, result)) {
+                tables.add(each);
+            }
+        }
+        return tables;
+    }
+    
+    private Collection<SimpleTableSegment> getTables(final ASTNode astNode) {
 
 Review comment:
   2. getTables(final ASTNode astNode) 

----------------------------------------------------------------
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] coveralls edited a comment on issue #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#issuecomment-614523955
 
 
   ## Pull Request Test Coverage Report for [Build 11106](https://coveralls.io/builds/30143544)
   
   * **48** of **80**   **(60.0%)**  changed or added relevant lines in **4** files are covered.
   * **126** unchanged lines in **15** files lost coverage.
   * Overall coverage increased (+**0.07%**) to **57.509%**
   
   ---
   
   |  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
   | :-----|--------------|--------|---: |
   | [sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/sharding/rewrite/token/generator/impl/OrderByTokenGenerator.java](https://coveralls.io/builds/30143544/source?filename=sharding-core%2Fsharding-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FOrderByTokenGenerator.java#L70) | 0 | 1 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/segment/select/projection/engine/ProjectionsContextEngine.java](https://coveralls.io/builds/30143544/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-binder%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fbinder%2Fsegment%2Fselect%2Fprojection%2Fengine%2FProjectionsContextEngine.java#L141) | 13 | 15 | 86.67%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/statement/dml/SelectStatementContext.java](https://coveralls.io/builds/30143544/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-binder%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fbinder%2Fstatement%2Fdml%2FSelectStatementContext.java#L195) | 32 | 61 | 52.46%
   <!-- | **Total:** | **48** | **80** | **60.0%** | -->
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [sharding-jdbc/sharding-jdbc-orchestration/src/main/java/org/apache/shardingsphere/shardingjdbc/orchestration/internal/datasource/AbstractOrchestrationDataSource.java](https://coveralls.io/builds/30143544/source?filename=sharding-jdbc%2Fsharding-jdbc-orchestration%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Forchestration%2Finternal%2Fdatasource%2FAbstractOrchestrationDataSource.java#L86) | 1 | 97.14% |
   | [sharding-proxy/sharding-proxy-frontend/sharding-proxy-frontend-mysql/src/main/java/org/apache/shardingsphere/shardingproxy/frontend/mysql/auth/MySQLAuthenticationEngine.java](https://coveralls.io/builds/30143544/source?filename=sharding-proxy%2Fsharding-proxy-frontend%2Fsharding-proxy-frontend-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingproxy%2Ffrontend%2Fmysql%2Fauth%2FMySQLAuthenticationEngine.java#L105) | 1 | 97.44% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/preparer/splitter/InventoryDataTaskSplitter.java](https://coveralls.io/builds/30143544/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fpreparer%2Fsplitter%2FInventoryDataTaskSplitter.java#L135) | 1 | 98.39% |
   | [shardingsphere-database-protocol/shardingsphere-database-protocol-mysql/src/main/java/org/apache/shardingsphere/database/protocol/mysql/payload/MySQLPacketPayload.java](https://coveralls.io/builds/30143544/source?filename=shardingsphere-database-protocol%2Fshardingsphere-database-protocol-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fdatabase%2Fprotocol%2Fmysql%2Fpayload%2FMySQLPacketPayload.java#L406) | 3 | 97.06% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/synctask/DefaultSyncTaskFactory.java](https://coveralls.io/builds/30143544/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fsynctask%2FDefaultSyncTaskFactory.java#L32) | 4 | 0% |
   | [sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/log/ConfigurationLogger.java](https://coveralls.io/builds/30143544/source?filename=sharding-core%2Fsharding-core-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fcore%2Flog%2FConfigurationLogger.java#L53) | 6 | 81.82% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/exception/ScalingJobNotFoundException.java](https://coveralls.io/builds/30143544/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fexception%2FScalingJobNotFoundException.java#L28) | 6 | 0% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/exception/SyncTaskExecuteException.java](https://coveralls.io/builds/30143544/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fexception%2FSyncTaskExecuteException.java#L28) | 6 | 0% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/controller/ScalingJobController.java](https://coveralls.io/builds/30143544/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fcontroller%2FScalingJobController.java#L58) | 8 | 0% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/synctask/inventory/InventoryDataSyncTask.java](https://coveralls.io/builds/30143544/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fsynctask%2Finventory%2FInventoryDataSyncTask.java#L99) | 8 | 76.92% |
   <!-- | **Total:** | **126** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/30143544/badge)](https://coveralls.io/builds/30143544) |
   | :-- | --: |
   | Change from base [Build 11082](https://coveralls.io/builds/30096394): |  0.07% |
   | Covered Lines: | 11894 |
   | Relevant Lines: | 20682 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
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 #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#discussion_r410070271
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/statement/dml/SelectStatementContext.java
 ##########
 @@ -93,10 +99,10 @@ public SelectStatementContext(final SelectStatement sqlStatement, final GroupByC
     
     public SelectStatementContext(final SchemaMetaData schemaMetaData, final String sql, final List<Object> parameters, final SelectStatement sqlStatement) {
         super(sqlStatement);
-        tablesContext = new TablesContext(sqlStatement.getSimpleTableSegments());
+        tablesContext = new TablesContext(getSimpleTableSegments(sqlStatement));
         groupByContext = new GroupByContextEngine().createGroupByContext(sqlStatement);
         orderByContext = new OrderByContextEngine().createOrderBy(sqlStatement, groupByContext);
-        projectionsContext = new ProjectionsContextEngine(schemaMetaData).createProjectionsContext(sql, sqlStatement, groupByContext, orderByContext);
+        projectionsContext = new ProjectionsContextEngine(schemaMetaData).createProjectionsContext(sql, this, groupByContext, orderByContext);
 
 Review comment:
   3. createProjectionsContext(sql, this, groupByContext, orderByContext);

----------------------------------------------------------------
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] jingshanglu commented on a change in pull request #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on a change in pull request #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#discussion_r410059159
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/statement/dml/SelectStatementContext.java
 ##########
 @@ -262,8 +268,86 @@ private boolean isTable(final OwnerSegment owner, final Collection<SimpleTableSe
         return true;
     }
     
+    private boolean isTable(final SimpleTableSegment owner, final Collection<SimpleTableSegment> tableSegments) {
+        for (SimpleTableSegment each : tableSegments) {
+            String tableName = owner.getTableName().getIdentifier().getValue();
+            if (tableName.equals(each.getAlias().orElse(null)) && !tableName.equals(each.getTableName().getIdentifier().getValue())) {
+                return false;
+            }
+        }
+        return true;
+    }
+    
     @Override
     public Optional<WhereSegment> getWhere() {
         return getSqlStatement().getWhere();
     }
+    
+    /**
+     * get tables.
+     *
+     * @param selectStatement SelectStatement.
+     *
+     * @return tables.
+     */
+    public Collection<SimpleTableSegment> getSimpleTableSegments(final SelectStatement selectStatement) {
+        Collection<SimpleTableSegment> result = getTables(selectStatement);
+        Collection<SimpleTableSegment> tables = new LinkedList<>();
+        for (SimpleTableSegment each : result) {
+            if (isTable(each, result)) {
+                tables.add(each);
+            }
+        }
+        return tables;
+    }
+    
+    private Collection<SimpleTableSegment> getTables(final ASTNode astNode) {
 
 Review comment:
   What do you mean? @tristaZero

----------------------------------------------------------------
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] coveralls commented on issue #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#issuecomment-614523955
 
 
   ## Pull Request Test Coverage Report for [Build 11088](https://coveralls.io/builds/30113623)
   
   * **48** of **77**   **(62.34%)**  changed or added relevant lines in **4** files are covered.
   * No unchanged relevant lines lost coverage.
   * Overall coverage increased (+**0.05%**) to **57.484%**
   
   ---
   
   |  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
   | :-----|--------------|--------|---: |
   | [sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/sharding/rewrite/token/generator/impl/OrderByTokenGenerator.java](https://coveralls.io/builds/30113623/source?filename=sharding-core%2Fsharding-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FOrderByTokenGenerator.java#L70) | 0 | 1 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/segment/select/projection/engine/ProjectionsContextEngine.java](https://coveralls.io/builds/30113623/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-binder%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fbinder%2Fsegment%2Fselect%2Fprojection%2Fengine%2FProjectionsContextEngine.java#L141) | 14 | 16 | 87.5%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/statement/dml/SelectStatementContext.java](https://coveralls.io/builds/30113623/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-binder%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fbinder%2Fstatement%2Fdml%2FSelectStatementContext.java#L196) | 31 | 57 | 54.39%
   <!-- | **Total:** | **48** | **77** | **62.34%** | -->
   
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/30113623/badge)](https://coveralls.io/builds/30113623) |
   | :-- | --: |
   | Change from base [Build 11082](https://coveralls.io/builds/30096394): |  0.05% |
   | Covered Lines: | 11863 |
   | Relevant Lines: | 20637 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
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] jingshanglu commented on a change in pull request #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on a change in pull request #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#discussion_r410058969
 
 

 ##########
 File path: encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
 ##########
 @@ -51,14 +53,15 @@
     
     @Override
     protected boolean isGenerateSQLTokenForEncrypt(final SQLStatementContext sqlStatementContext) {
-        return sqlStatementContext instanceof SelectStatementContext && !((SelectStatementContext) sqlStatementContext).getSqlStatement().getSimpleTableSegments().isEmpty();
+        SQLStatement sqlStatement = sqlStatementContext.getSqlStatement();
+        return sqlStatementContext instanceof SelectStatementContext && !((SelectStatementContext) sqlStatementContext).getSimpleTableSegments((SelectStatement) sqlStatement).isEmpty();
 
 Review comment:
   What do you mean? @tristaZero 

----------------------------------------------------------------
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] jingshanglu commented on issue #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#issuecomment-615202744
 
 
   ok,It has been modified。 @tristaZero 

----------------------------------------------------------------
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 #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#discussion_r410153576
 
 

 ##########
 File path: encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
 ##########
 @@ -51,14 +52,15 @@
     
     @Override
     protected boolean isGenerateSQLTokenForEncrypt(final SQLStatementContext sqlStatementContext) {
-        return sqlStatementContext instanceof SelectStatementContext && !((SelectStatementContext) sqlStatementContext).getSqlStatement().getSimpleTableSegments().isEmpty();
+        SQLStatement sqlStatement = sqlStatementContext.getSqlStatement();
 
 Review comment:
   Not used?

----------------------------------------------------------------
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] codecov-io commented on issue #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#issuecomment-615227411
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206?src=pr&el=h1) Report
   > Merging [#5206](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-shardingsphere/commit/d84a21ec63ee0b8ea3783fc1da5accb65663000b&el=desc) will **increase** coverage by `0.01%`.
   > The diff coverage is `41.77%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so)](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #5206      +/-   ##
   ============================================
   + Coverage     54.00%   54.01%   +0.01%     
     Complexity      408      408              
   ============================================
     Files          1159     1162       +3     
     Lines         20601    20686      +85     
     Branches       3708     3731      +23     
   ============================================
   + Hits          11126    11174      +48     
   - Misses         8782     8806      +24     
   - Partials        693      706      +13     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...te/token/generator/impl/OrderByTokenGenerator.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree#diff-c2hhcmRpbmctY29yZS9zaGFyZGluZy1jb3JlLXJld3JpdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NoYXJkaW5nL3Jld3JpdGUvdG9rZW4vZ2VuZXJhdG9yL2ltcGwvT3JkZXJCeVRva2VuR2VuZXJhdG9yLmphdmE=) | `57.14% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../sql/parser/sql/statement/dml/SelectStatement.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXN0YXRlbWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc3RhdGVtZW50L2RtbC9TZWxlY3RTdGF0ZW1lbnQuamF2YQ==) | `0.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...r/binder/statement/dml/SelectStatementContext.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWJpbmRlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9iaW5kZXIvc3RhdGVtZW50L2RtbC9TZWxlY3RTdGF0ZW1lbnRDb250ZXh0LmphdmE=) | `45.28% <40.98%> (-2.34%)` | `0.00 <0.00> (ø)` | |
   | [...ct/projection/engine/ProjectionsContextEngine.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWJpbmRlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9iaW5kZXIvc2VnbWVudC9zZWxlY3QvcHJvamVjdGlvbi9lbmdpbmUvUHJvamVjdGlvbnNDb250ZXh0RW5naW5lLmphdmE=) | `59.45% <46.66%> (-0.55%)` | `0.00 <0.00> (ø)` | |
   | [...enerator/impl/EncryptProjectionTokenGenerator.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree#diff-ZW5jcnlwdC1jb3JlL2VuY3J5cHQtY29yZS1yZXdyaXRlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbmNyeXB0L3Jld3JpdGUvdG9rZW4vZ2VuZXJhdG9yL2ltcGwvRW5jcnlwdFByb2plY3Rpb25Ub2tlbkdlbmVyYXRvci5qYXZh) | `82.50% <50.00%> (ø)` | `1.00 <0.00> (ø)` | |
   | [...e/shardingsphere/core/log/ConfigurationLogger.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree#diff-c2hhcmRpbmctY29yZS9zaGFyZGluZy1jb3JlLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvY29yZS9sb2cvQ29uZmlndXJhdGlvbkxvZ2dlci5qYXZh) | `60.60% <0.00%> (-10.83%)` | `0.00% <0.00%> (ø%)` | |
   | [...al/datasource/OrchestrationShardingDataSource.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree#diff-c2hhcmRpbmctamRiYy9zaGFyZGluZy1qZGJjLW9yY2hlc3RyYXRpb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NoYXJkaW5namRiYy9vcmNoZXN0cmF0aW9uL2ludGVybmFsL2RhdGFzb3VyY2UvT3JjaGVzdHJhdGlvblNoYXJkaW5nRGF0YVNvdXJjZS5qYXZh) | `84.78% <0.00%> (-10.09%)` | `0.00% <0.00%> (ø%)` | |
   | [...l/mysql/packet/handshake/MySQLHandshakePacket.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZGF0YWJhc2UtcHJvdG9jb2wvc2hhcmRpbmdzcGhlcmUtZGF0YWJhc2UtcHJvdG9jb2wtbXlzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGFiYXNlL3Byb3RvY29sL215c3FsL3BhY2tldC9oYW5kc2hha2UvTXlTUUxIYW5kc2hha2VQYWNrZXQuamF2YQ==) | `81.48% <0.00%> (-7.41%)` | `1.00% <0.00%> (ø%)` | |
   | [...ase/protocol/mysql/payload/MySQLPacketPayload.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZGF0YWJhc2UtcHJvdG9jb2wvc2hhcmRpbmdzcGhlcmUtZGF0YWJhc2UtcHJvdG9jb2wtbXlzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGFiYXNlL3Byb3RvY29sL215c3FsL3BheWxvYWQvTXlTUUxQYWNrZXRQYXlsb2FkLmphdmE=) | `97.05% <0.00%> (-2.95%)` | `0.00% <0.00%> (ø%)` | |
   | [...cket/handshake/MySQLHandshakeResponse41Packet.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtZGF0YWJhc2UtcHJvdG9jb2wvc2hhcmRpbmdzcGhlcmUtZGF0YWJhc2UtcHJvdG9jb2wtbXlzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGFiYXNlL3Byb3RvY29sL215c3FsL3BhY2tldC9oYW5kc2hha2UvTXlTUUxIYW5kc2hha2VSZXNwb25zZTQxUGFja2V0LmphdmE=) | `95.83% <0.00%> (-2.09%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [24 more](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206?src=pr&el=footer). Last update [d84a21e...28c00e2](https://codecov.io/gh/apache/incubator-shardingsphere/pull/5206?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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 #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#discussion_r410153784
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/segment/select/projection/engine/ProjectionsContextEngine.java
 ##########
 @@ -58,17 +57,20 @@ public ProjectionsContextEngine(final SchemaMetaData schemaMetaData) {
      * Create projections context.
      *
      * @param sql SQL
-     * @param selectStatement SQL statement
+     * @param tables tables
+     * @param projectionsSegment projection Segments
      * @param groupByContext group by context
      * @param orderByContext order by context
      * @return projections context
      */
-    public ProjectionsContext createProjectionsContext(final String sql, final SelectStatement selectStatement, final GroupByContext groupByContext, final OrderByContext orderByContext) {
-        ProjectionsSegment projectionsSegment = selectStatement.getProjections();
-        Collection<Projection> projections = getProjections(sql, selectStatement.getSimpleTableSegments(), projectionsSegment);
+    public ProjectionsContext createProjectionsContext(final String sql, final Collection<SimpleTableSegment> tables, final ProjectionsSegment projectionsSegment,
+                                                       final GroupByContext groupByContext, final OrderByContext orderByContext) {
+//        SelectStatement selectStatement = context.getSqlStatement();
 
 Review comment:
   Please remove them.

----------------------------------------------------------------
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] coveralls edited a comment on issue #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#issuecomment-614523955
 
 
   ## Pull Request Test Coverage Report for [Build 11113](https://coveralls.io/builds/30145569)
   
   * **47** of **79**   **(59.49%)**  changed or added relevant lines in **4** files are covered.
   * **131** unchanged lines in **16** files lost coverage.
   * Overall coverage increased (+**0.06%**) to **57.493%**
   
   ---
   
   |  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
   | :-----|--------------|--------|---: |
   | [sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/sharding/rewrite/token/generator/impl/OrderByTokenGenerator.java](https://coveralls.io/builds/30145569/source?filename=sharding-core%2Fsharding-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FOrderByTokenGenerator.java#L70) | 0 | 1 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/segment/select/projection/engine/ProjectionsContextEngine.java](https://coveralls.io/builds/30145569/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-binder%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fbinder%2Fsegment%2Fselect%2Fprojection%2Fengine%2FProjectionsContextEngine.java#L139) | 13 | 15 | 86.67%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/statement/dml/SelectStatementContext.java](https://coveralls.io/builds/30145569/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-binder%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fbinder%2Fstatement%2Fdml%2FSelectStatementContext.java#L195) | 32 | 61 | 52.46%
   <!-- | **Total:** | **47** | **79** | **59.49%** | -->
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [sharding-jdbc/sharding-jdbc-orchestration/src/main/java/org/apache/shardingsphere/shardingjdbc/orchestration/internal/datasource/AbstractOrchestrationDataSource.java](https://coveralls.io/builds/30145569/source?filename=sharding-jdbc%2Fsharding-jdbc-orchestration%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Forchestration%2Finternal%2Fdatasource%2FAbstractOrchestrationDataSource.java#L86) | 1 | 97.14% |
   | [sharding-proxy/sharding-proxy-frontend/sharding-proxy-frontend-mysql/src/main/java/org/apache/shardingsphere/shardingproxy/frontend/mysql/auth/MySQLAuthenticationEngine.java](https://coveralls.io/builds/30145569/source?filename=sharding-proxy%2Fsharding-proxy-frontend%2Fsharding-proxy-frontend-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingproxy%2Ffrontend%2Fmysql%2Fauth%2FMySQLAuthenticationEngine.java#L105) | 1 | 97.44% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/preparer/splitter/InventoryDataTaskSplitter.java](https://coveralls.io/builds/30145569/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fpreparer%2Fsplitter%2FInventoryDataTaskSplitter.java#L135) | 1 | 98.39% |
   | [shardingsphere-database-protocol/shardingsphere-database-protocol-mysql/src/main/java/org/apache/shardingsphere/database/protocol/mysql/payload/MySQLPacketPayload.java](https://coveralls.io/builds/30145569/source?filename=shardingsphere-database-protocol%2Fshardingsphere-database-protocol-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fdatabase%2Fprotocol%2Fmysql%2Fpayload%2FMySQLPacketPayload.java#L406) | 3 | 97.06% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/synctask/DefaultSyncTaskFactory.java](https://coveralls.io/builds/30145569/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fsynctask%2FDefaultSyncTaskFactory.java#L32) | 4 | 0% |
   | [sharding-jdbc/sharding-jdbc-orchestration/src/main/java/org/apache/shardingsphere/shardingjdbc/orchestration/internal/datasource/OrchestrationShardingDataSource.java](https://coveralls.io/builds/30145569/source?filename=sharding-jdbc%2Fsharding-jdbc-orchestration%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Forchestration%2Finternal%2Fdatasource%2FOrchestrationShardingDataSource.java#L91) | 5 | 89.13% |
   | [sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/log/ConfigurationLogger.java](https://coveralls.io/builds/30145569/source?filename=sharding-core%2Fsharding-core-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fcore%2Flog%2FConfigurationLogger.java#L53) | 6 | 81.82% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/exception/ScalingJobNotFoundException.java](https://coveralls.io/builds/30145569/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fexception%2FScalingJobNotFoundException.java#L28) | 6 | 0% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/exception/SyncTaskExecuteException.java](https://coveralls.io/builds/30145569/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fexception%2FSyncTaskExecuteException.java#L28) | 6 | 0% |
   | [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/controller/ScalingJobController.java](https://coveralls.io/builds/30145569/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fcontroller%2FScalingJobController.java#L58) | 8 | 0% |
   <!-- | **Total:** | **131** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/30145569/badge)](https://coveralls.io/builds/30145569) |
   | :-- | --: |
   | Change from base [Build 11082](https://coveralls.io/builds/30096394): |  0.06% |
   | Covered Lines: | 11893 |
   | Relevant Lines: | 20686 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
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 #5206: Simplify the SelectStatement

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5206: Simplify the SelectStatement
URL: https://github.com/apache/incubator-shardingsphere/pull/5206#discussion_r410154830
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/statement/dml/SelectStatementContext.java
 ##########
 @@ -262,8 +267,97 @@ private boolean isTable(final OwnerSegment owner, final Collection<SimpleTableSe
         return true;
     }
     
+    private boolean isTable(final SimpleTableSegment owner, final Collection<SimpleTableSegment> tableSegments) {
+        for (SimpleTableSegment each : tableSegments) {
+            String tableName = owner.getTableName().getIdentifier().getValue();
+            if (tableName.equals(each.getAlias().orElse(null)) && !tableName.equals(each.getTableName().getIdentifier().getValue())) {
+                return false;
+            }
+        }
+        return true;
+    }
+    
     @Override
     public Optional<WhereSegment> getWhere() {
         return getSqlStatement().getWhere();
     }
+    
+    /**
+     * get tables.
+     * @return tables.
+     */
+    public Collection<SimpleTableSegment> getSimpleTableSegments() {
+        Collection<SimpleTableSegment> result = getTables();
+        Collection<SimpleTableSegment> tables = new LinkedList<>();
 
 Review comment:
   Please exchange two of these names, since `result` is expected to return.

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