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