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/03/26 11:29:59 UTC

[GitHub] [incubator-shardingsphere] jingshanglu opened a new pull request #4961: fix selectStatement contained tableReferences for other db

jingshanglu opened a new pull request #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961
 
 
   ref #4885 .
   
   Changes proposed in this pull request:
   - fix selectStatement contained tableReferences for other db
   

----------------------------------------------------------------
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 issue #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-605826691
 
 
   Good job, and it is time to review the SQL cases below whether it could cover all the databases except for `MySQL`(They are updated  in your last PR).
   > 1. select_inner_join_related_with_name
    2. select_sharding_route_with_broadcast_table

----------------------------------------------------------------
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 #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on a change in pull request #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#discussion_r399025057
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dml/SelectStatement.java
 ##########
 @@ -41,10 +40,10 @@
 @Setter
 public final class SelectStatement extends DMLStatement {
     
-    private final Collection<TableSegment> tables = new LinkedList<>();
-    
     private ProjectionsSegment projections;
     
+    private final Collection<TableSegment> tables = new LinkedList<>();
+    
 
 Review comment:
   It has recovered.

----------------------------------------------------------------
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 #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-604845406
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961?src=pr&el=h1) Report
   > Merging [#4961](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-shardingsphere/commit/4d53d222d3195f78ad1054c3f530f43c764452b6&el=desc) will **decrease** coverage by `0.23%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so)](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #4961      +/-   ##
   ============================================
   - Coverage     54.45%   54.22%   -0.24%     
   - Complexity      440      443       +3     
   ============================================
     Files          1140     1143       +3     
     Lines         20482    20560      +78     
     Branches       3765     3760       -5     
   ============================================
   - Hits          11154    11148       -6     
   - Misses         8620     8706      +86     
   + Partials        708      706       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...l/parser/oracle/visitor/impl/OracleDMLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWRpYWxlY3Qvc2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci1vcmFjbGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NxbC9wYXJzZXIvb3JhY2xlL3Zpc2l0b3IvaW1wbC9PcmFjbGVETUxWaXNpdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../postgresql/visitor/impl/PostgreSQLDMLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWRpYWxlY3Qvc2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci1wb3N0Z3Jlc3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zcWwvcGFyc2VyL3Bvc3RncmVzcWwvdmlzaXRvci9pbXBsL1Bvc3RncmVTUUxETUxWaXNpdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...sql/parser/sql92/visitor/impl/SQL92DMLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWRpYWxlY3Qvc2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci1zcWw5Mi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWw5Mi92aXNpdG9yL2ltcGwvU1FMOTJETUxWaXNpdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...er/sqlserver/visitor/impl/SQLServerDMLVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWRpYWxlY3Qvc2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci1zcWxzZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NxbC9wYXJzZXIvc3Fsc2VydmVyL3Zpc2l0b3IvaW1wbC9TUUxTZXJ2ZXJETUxWaXNpdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../sql/parser/sql/statement/dml/SelectStatement.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXN0YXRlbWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc3RhdGVtZW50L2RtbC9TZWxlY3RTdGF0ZW1lbnQuamF2YQ==) | `0.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...che/shardingsphere/core/rule/BindingTableRule.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree#diff-c2hhcmRpbmctY29yZS9zaGFyZGluZy1jb3JlLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvY29yZS9ydWxlL0JpbmRpbmdUYWJsZVJ1bGUuamF2YQ==) | `73.07% <0.00%> (-26.93%)` | `0.00% <0.00%> (ø%)` | |
   | [...underlying/common/database/type/DatabaseTypes.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtdW5kZXJseWluZy9zaGFyZGluZ3NwaGVyZS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3VuZGVybHlpbmcvY29tbW9uL2RhdGFiYXNlL3R5cGUvRGF0YWJhc2VUeXBlcy5qYXZh) | `83.33% <0.00%> (-7.15%)` | `0.00% <0.00%> (ø%)` | |
   | [.../apache/shardingsphere/core/rule/ShardingRule.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree#diff-c2hhcmRpbmctY29yZS9zaGFyZGluZy1jb3JlLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvY29yZS9ydWxlL1NoYXJkaW5nUnVsZS5qYXZh) | `74.34% <0.00%> (-5.24%)` | `0.00% <0.00%> (ø%)` | |
   | [.../shardingsphere/core/shard/BaseShardingEngine.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree#diff-c2hhcmRpbmctY29yZS9zaGFyZGluZy1jb3JlLWVudHJ5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9jb3JlL3NoYXJkL0Jhc2VTaGFyZGluZ0VuZ2luZS5qYXZh) | `83.33% <0.00%> (-3.04%)` | `0.00% <0.00%> (ø%)` | |
   | [...rdingproxy/config/ShardingConfigurationLoader.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree#diff-c2hhcmRpbmctcHJveHkvc2hhcmRpbmctcHJveHktY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zaGFyZGluZ3Byb3h5L2NvbmZpZy9TaGFyZGluZ0NvbmZpZ3VyYXRpb25Mb2FkZXIuamF2YQ==) | `85.71% <0.00%> (-1.39%)` | `1.00% <0.00%> (ø%)` | |
   | ... and [47 more](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961?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/4961?src=pr&el=footer). Last update [4d53d22...a437951](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4961?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] coveralls commented on issue #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-604846960
 
 
   ## Pull Request Test Coverage Report for [Build 10639](https://coveralls.io/builds/29654518)
   
   * **0** of **192**   **(0.0%)**  changed or added relevant lines in **4** files are covered.
   * **109** unchanged lines in **11** files lost coverage.
   * Overall coverage decreased (**-0.1%**) to **57.714%**
   
   ---
   
   |  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
   | :-----|--------------|--------|---: |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sql92/src/main/java/org/apache/shardingsphere/sql/parser/sql92/visitor/impl/SQL92DMLVisitor.java](https://coveralls.io/builds/29654518/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-sql92%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql92%2Fvisitor%2Fimpl%2FSQL92DMLVisitor.java#L139) | 0 | 45 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/java/org/apache/shardingsphere/sql/parser/oracle/visitor/impl/OracleDMLVisitor.java](https://coveralls.io/builds/29654518/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-oracle%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Foracle%2Fvisitor%2Fimpl%2FOracleDMLVisitor.java#L145) | 0 | 49 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/postgresql/visitor/impl/PostgreSQLDMLVisitor.java](https://coveralls.io/builds/29654518/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fpostgresql%2Fvisitor%2Fimpl%2FPostgreSQLDMLVisitor.java#L151) | 0 | 49 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sqlserver/src/main/java/org/apache/shardingsphere/sql/parser/sqlserver/visitor/impl/SQLServerDMLVisitor.java](https://coveralls.io/builds/29654518/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-sqlserver%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsqlserver%2Fvisitor%2Fimpl%2FSQLServerDMLVisitor.java#L140) | 0 | 49 | 0.0%
   <!-- | **Total:** | **0** | **192** | **0.0%** | -->
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sql92/src/main/java/org/apache/shardingsphere/sql/parser/sql92/visitor/impl/SQL92DMLVisitor.java](https://coveralls.io/builds/29654518/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-sql92%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql92%2Fvisitor%2Fimpl%2FSQL92DMLVisitor.java#L378) | 2 | 0% |
   | [shardingsphere-underlying/shardingsphere-common/src/main/java/org/apache/shardingsphere/underlying/common/database/type/DatabaseTypes.java](https://coveralls.io/builds/29654518/source?filename=shardingsphere-underlying%2Fshardingsphere-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Funderlying%2Fcommon%2Fdatabase%2Ftype%2FDatabaseTypes.java#L32) | 2 | 83.33% |
   | [sharding-core/sharding-core-entry/src/main/java/org/apache/shardingsphere/core/shard/BaseShardingEngine.java](https://coveralls.io/builds/29654518/source?filename=sharding-core%2Fsharding-core-entry%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fcore%2Fshard%2FBaseShardingEngine.java#L120) | 3 | 91.67% |
   | [sharding-orchestration/sharding-orchestration-core/sharding-orchestration-core-registrycenter/src/main/java/org/apache/shardingsphere/orchestration/core/registrycenter/util/IpUtils.java](https://coveralls.io/builds/29654518/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsharding-orchestration-core-registrycenter%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Fcore%2Fregistrycenter%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/java/org/apache/shardingsphere/sql/parser/oracle/visitor/impl/OracleDMLVisitor.java](https://coveralls.io/builds/29654518/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-oracle%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Foracle%2Fvisitor%2Fimpl%2FOracleDMLVisitor.java#L225) | 3 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/postgresql/visitor/impl/PostgreSQLDMLVisitor.java](https://coveralls.io/builds/29654518/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fpostgresql%2Fvisitor%2Fimpl%2FPostgreSQLDMLVisitor.java#L230) | 3 | 0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sqlserver/src/main/java/org/apache/shardingsphere/sql/parser/sqlserver/visitor/impl/SQLServerDMLVisitor.java](https://coveralls.io/builds/29654518/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-sqlserver%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsqlserver%2Fvisitor%2Fimpl%2FSQLServerDMLVisitor.java#L220) | 3 | 0% |
   | [sharding-jdbc/sharding-jdbc-core/src/main/java/org/apache/shardingsphere/shardingjdbc/jdbc/core/statement/ShardingStatement.java](https://coveralls.io/builds/29654518/source?filename=sharding-jdbc%2Fsharding-jdbc-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Fjdbc%2Fcore%2Fstatement%2FShardingStatement.java#L273) | 4 | 70.16% |
   | [sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/rule/ShardingRule.java](https://coveralls.io/builds/29654518/source?filename=sharding-core%2Fsharding-core-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fcore%2Frule%2FShardingRule.java#L499) | 6 | 79.61% |
   | [shardingsphere-underlying/shardingsphere-rewrite/shardingsphere-rewrite-engine/src/main/java/org/apache/shardingsphere/underlying/rewrite/SQLRewriteEntry.java](https://coveralls.io/builds/29654518/source?filename=shardingsphere-underlying%2Fshardingsphere-rewrite%2Fshardingsphere-rewrite-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Funderlying%2Frewrite%2FSQLRewriteEntry.java#L54) | 12 | 0% |
   <!-- | **Total:** | **109** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/29654518/badge)](https://coveralls.io/builds/29654518) |
   | :-- | --: |
   | Change from base [Build 1177](https://coveralls.io/builds/29628541): |  -0.1% |
   | Covered Lines: | 11866 |
   | Relevant Lines: | 20560 |
   
   ---
   ##### 💛  - [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 issue #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-604801372
 
 
   Test of binder module use tables of selectStatement, cannot remove  tables.  @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 edited a comment on issue #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-604803095
 
 
   @jingshanglu You mean `tables` will be removed in the future, not now?

----------------------------------------------------------------
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 edited a comment on issue #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
jingshanglu edited a comment on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-604848977
 
 
   Not only the test problem, but the call to the business code in the test. @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 #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-604806834
 
 
   > @jingshanglu You mean `tables` will be removed in the future, not now?
   
   The premise is that the binder module needs to be 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] coveralls edited a comment on issue #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-604846960
 
 
   ## Pull Request Test Coverage Report for [Build 10701](https://coveralls.io/builds/29699367)
   
   * **1** of **191**   **(0.52%)**  changed or added relevant lines in **6** files are covered.
   * **729** unchanged lines in **39** files lost coverage.
   * Overall coverage decreased (**-0.6%**) to **57.178%**
   
   ---
   
   |  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
   | :-----|--------------|--------|---: |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dml/SelectStatement.java](https://coveralls.io/builds/29699367/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-statement%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fdml%2FSelectStatement.java#L110) | 0 | 2 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sql92/src/main/java/org/apache/shardingsphere/sql/parser/sql92/visitor/impl/SQL92DMLVisitor.java](https://coveralls.io/builds/29699367/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-sql92%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql92%2Fvisitor%2Fimpl%2FSQL92DMLVisitor.java#L139) | 0 | 44 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/java/org/apache/shardingsphere/sql/parser/oracle/visitor/impl/OracleDMLVisitor.java](https://coveralls.io/builds/29699367/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-oracle%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Foracle%2Fvisitor%2Fimpl%2FOracleDMLVisitor.java#L145) | 0 | 48 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/postgresql/visitor/impl/PostgreSQLDMLVisitor.java](https://coveralls.io/builds/29699367/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fpostgresql%2Fvisitor%2Fimpl%2FPostgreSQLDMLVisitor.java#L151) | 0 | 48 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sqlserver/src/main/java/org/apache/shardingsphere/sql/parser/sqlserver/visitor/impl/SQLServerDMLVisitor.java](https://coveralls.io/builds/29699367/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-sqlserver%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsqlserver%2Fvisitor%2Fimpl%2FSQLServerDMLVisitor.java#L140) | 0 | 48 | 0.0%
   <!-- | **Total:** | **1** | **191** | **0.52%** | -->
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/context/EncryptSQLRewriteContextDecorator.java](https://coveralls.io/builds/29699367/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Fcontext%2FEncryptSQLRewriteContextDecorator.java#L48) | 1 | 88.89% |
   | [master-slave-core/master-slave-core-route/src/main/java/org/apache/shardingsphere/masterslave/route/engine/MasterSlaveRouteDecorator.java](https://coveralls.io/builds/29699367/source?filename=master-slave-core%2Fmaster-slave-core-route%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fmasterslave%2Froute%2Fengine%2FMasterSlaveRouteDecorator.java#L63) | 1 | 94.44% |
   | [shadow-core/shadow-core-rewrite/src/main/java/org/apache/shardingsphere/shadow/rewrite/context/ShadowSQLRewriteContextDecorator.java](https://coveralls.io/builds/29699367/source?filename=shadow-core%2Fshadow-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshadow%2Frewrite%2Fcontext%2FShadowSQLRewriteContextDecorator.java#L46) | 1 | 0% |
   | [sharding-core/sharding-core-merge/src/main/java/org/apache/shardingsphere/sharding/merge/ShardingResultMergerEngine.java](https://coveralls.io/builds/29699367/source?filename=sharding-core%2Fsharding-core-merge%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Fmerge%2FShardingResultMergerEngine.java#L50) | 1 | 85.71% |
   | [sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/sharding/rewrite/context/ShardingSQLRewriteContextDecorator.java](https://coveralls.io/builds/29699367/source?filename=sharding-core%2Fsharding-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Frewrite%2Fcontext%2FShardingSQLRewriteContextDecorator.java#L52) | 1 | 77.78% |
   | [sharding-core/sharding-core-route/src/main/java/org/apache/shardingsphere/sharding/route/engine/ShardingRouteDecorator.java](https://coveralls.io/builds/29699367/source?filename=sharding-core%2Fsharding-core-route%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Froute%2Fengine%2FShardingRouteDecorator.java#L157) | 1 | 31.58% |
   | [sharding-jdbc/sharding-jdbc-core/src/main/java/org/apache/shardingsphere/shardingjdbc/jdbc/core/datasource/EncryptDataSource.java](https://coveralls.io/builds/29699367/source?filename=sharding-jdbc%2Fsharding-jdbc-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Fjdbc%2Fcore%2Fdatasource%2FEncryptDataSource.java#L36) | 1 | 88.89% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-sql92/src/main/java/org/apache/shardingsphere/sql/parser/sql92/visitor/impl/SQL92DMLVisitor.java](https://coveralls.io/builds/29699367/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-dialect%2Fshardingsphere-sql-parser-sql92%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql92%2Fvisitor%2Fimpl%2FSQL92DMLVisitor.java#L377) | 2 | 0% |
   | [shardingsphere-underlying/shardingsphere-common/src/main/java/org/apache/shardingsphere/underlying/common/database/type/DatabaseTypes.java](https://coveralls.io/builds/29699367/source?filename=shardingsphere-underlying%2Fshardingsphere-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Funderlying%2Fcommon%2Fdatabase%2Ftype%2FDatabaseTypes.java#L32) | 2 | 83.33% |
   | [encrypt-core/encrypt-core-merge/src/main/java/org/apache/shardingsphere/encrypt/merge/dal/EncryptDALResultDecorator.java](https://coveralls.io/builds/29699367/source?filename=encrypt-core%2Fencrypt-core-merge%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Fmerge%2Fdal%2FEncryptDALResultDecorator.java#L39) | 3 | 0% |
   <!-- | **Total:** | **729** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/29699367/badge)](https://coveralls.io/builds/29699367) |
   | :-- | --: |
   | Change from base [Build 1177](https://coveralls.io/builds/29628541): |  -0.6% |
   | Covered Lines: | 11730 |
   | Relevant Lines: | 20515 |
   
   ---
   ##### 💛  - [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 merged pull request #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
tristaZero merged pull request #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961
 
 
   

----------------------------------------------------------------
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 #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#discussion_r399022803
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dml/SelectStatement.java
 ##########
 @@ -118,19 +117,4 @@
         }
         return result;
     }
-    
-    /**
-     * Get subQuery table segments.
-     *
-     * @return subQuery table segments
-     */
-    public Collection<SubqueryTableSegment> getSubQueryTableSegments() {
 
 Review comment:
   Why did you remove this function?

----------------------------------------------------------------
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 #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-604848977
 
 
   Modify the binder module test in the next subtask, how do you think? @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 edited a comment on issue #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
jingshanglu edited a comment on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-604848977
 
 
   Not only the test problem, but the call to the business code in the test, the business code uses the tables property @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 #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#discussion_r399021863
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dml/SelectStatement.java
 ##########
 @@ -41,10 +40,10 @@
 @Setter
 public final class SelectStatement extends DMLStatement {
     
-    private final Collection<TableSegment> tables = new LinkedList<>();
-    
     private ProjectionsSegment projections;
     
+    private final Collection<TableSegment> tables = new LinkedList<>();
+    
 
 Review comment:
   Sorry, `tables` was just moved below?

----------------------------------------------------------------
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 edited a comment on issue #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-605826691
 
 
   Good job, and it is time to review the SQL cases below whether it could cover all the databases except for `MySQL`(They are updated  in your last PR).
   > 1. select_inner_join_related_with_name 2. select_sharding_route_with_broadcast_table

----------------------------------------------------------------
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 #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on a change in pull request #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#discussion_r399024228
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dml/SelectStatement.java
 ##########
 @@ -118,19 +117,4 @@
         }
         return result;
     }
-    
-    /**
-     * Get subQuery table segments.
-     *
-     * @return subQuery table segments
-     */
-    public Collection<SubqueryTableSegment> getSubQueryTableSegments() {
 
 Review comment:
   This func not be 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] tristaZero commented on issue #4961: fix selectStatement contained tableReferences for other db

Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #4961: fix selectStatement contained tableReferences for other db
URL: https://github.com/apache/incubator-shardingsphere/pull/4961#issuecomment-604803095
 
 
   @jingshanglu You means `tables` will be removed in the future, not now?

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