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/02/25 04:15:38 UTC
[GitHub] [incubator-shardingsphere] yuzel opened a new pull request #4456:
#4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
yuzel opened a new pull request #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456
Fixes #4406.
Changes proposed in this pull request:
- add parse Integer, Double, Decimal
-
-
----------------------------------------------------------------
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
#4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456#issuecomment-590684265
## Pull Request Test Coverage Report for [Build 9816](https://coveralls.io/builds/28978503)
* **0** of **0** changed or added relevant lines in **0** files are covered.
* **3** unchanged lines in **1** file lost coverage.
* Overall coverage decreased (**-0.006%**) to **58.878%**
---
| Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/28978503/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
<!-- | **Total:** | **3** | | -->
| Totals | [](https://coveralls.io/builds/28978503) |
| :-- | --: |
| Change from base [Build 938](https://coveralls.io/builds/28971920): | -0.006% |
| Covered Lines: | 10614 |
| Relevant Lines: | 18027 |
---
##### 💛 - [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 #4456:
#4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
tristaZero merged pull request #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456
----------------------------------------------------------------
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 #4456:
#4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456#issuecomment-590795865
Hi @yuzel Your PR told me that you have a good understand of `g4` flie and `ANTLR`. Thanks for this PR.
After my investigation, the root cause for issue #4406 is that the wrong definition of `dataTypeName` in [BaseRule.g4](https://github.com/apache/incubator-shardingsphere/blob/master/shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/antlr4/imports/mysql/BaseRule.g4). In other words, it is supposed to enumerate all the names of data type, rather than use `identifier` to define `dataTypeName`.
Besides, `DECIMAL`, `DOUBLE` and ` INTEGER` is considered as [reserved word](https://dev.mysql.com/doc/refman/8.0/en/keywords.html). Based on that, i think there are some more modifications for you to do before it is merged. Please look at the following items,
- [ ] Correct the wrong definition of `dataTypeName` in [BaseRule.g4](https://github.com/apache/incubator-shardingsphere/blob/master/shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/antlr4/imports/mysql/BaseRule.g4). ([Antlr MySQL](https://github.com/mysql/mysql-workbench/blob/d6dcc4254205a2a8aff01efd9aa9d1afd2711ee5/library/parsers/grammars/MySQLParser.g4#L3192) is a good reference. 😃 )
- [ ] Delete modification of `unreservedWord` in your PR
- [ ] Keep the existing test cases.
Welcome your opinion, and if you need any help, please contact me.
Looking forward to your reply.
Trista
----------------------------------------------------------------
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 #4456:
#4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456#issuecomment-591761450
@yuzel Hi i like your PR, which is clear and including many unit tests.
Just out of my curiosity, how do you differentiate the `Keyword` and `MySQLKeyword`? Is there any document or link as reference?
----------------------------------------------------------------
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 #4456:
#4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456#issuecomment-590684265
## Pull Request Test Coverage Report for [Build 9772](https://coveralls.io/builds/28938032)
* **0** of **0** changed or added relevant lines in **0** files are covered.
* **3** unchanged lines in **1** file lost coverage.
* Overall coverage decreased (**-0.005%**) to **58.153%**
---
| Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/28938032/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
<!-- | **Total:** | **3** | | -->
| Totals | [](https://coveralls.io/builds/28938032) |
| :-- | --: |
| Change from base [Build 925](https://coveralls.io/builds/28933816): | -0.005% |
| Covered Lines: | 10613 |
| Relevant Lines: | 18250 |
---
##### 💛 - [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 issue #4456:
#4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456#issuecomment-591225588
@yuzel Hi i am glad you can continue optimizing this PR.
As you said, we should review the definition of `dataTypeName` of `PostgreSQL`, `SQLServer` , `Oracle` and `SQL92` as well and include the corresponding `dataTypeName`, like `Complex and Array`.
It seems not an easy job, so my suggestion is that we can make `dataTypeName` of `MySQL` right firstly and make this PR merged ASAP.
----------------------------------------------------------------
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] xiaolianghu commented on issue #4456:
#4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
xiaolianghu commented on issue #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456#issuecomment-604938355
When is version 4.1.0 scheduled for release? 3qs!
----------------------------------------------------------------
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 #4456:
#4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456#issuecomment-591229153
@yuzel Hi
I created issue #4482 to record all the tasks concerning `DataTypeName`, hope see the fist item of this issue is done by YOUR PR. 😉
After that, if you are interested in other ones, please be free to move forward!
----------------------------------------------------------------
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
#4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456#issuecomment-590684265
## Pull Request Test Coverage Report for [Build 1893](https://coveralls.io/builds/28989085)
* **0** of **0** changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**0.006%**) to **58.889%**
---
| Totals | [](https://coveralls.io/builds/28989085) |
| :-- | --: |
| Change from base [Build 938](https://coveralls.io/builds/28971920): | 0.006% |
| Covered Lines: | 10616 |
| Relevant Lines: | 18027 |
---
##### 💛 - [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] yuzel commented on issue #4456: #4406
fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
yuzel commented on issue #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456#issuecomment-591215980
> Please look at my comment, thx.
OK, I will modify dataTypeName to the type supported by the database. But I found that postgresql supports complex and array. Do we need to support it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] yuzel commented on issue #4456: #4406
fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
Posted by GitBox <gi...@apache.org>.
yuzel commented on issue #4456: #4406 fix MySQL, PostgreSQL DDL cannot parse Integer, Double, Decimal
URL: https://github.com/apache/incubator-shardingsphere/pull/4456#issuecomment-591774482
> @yuzel Hi
> I created issue #4482 to record all the tasks concerning `DataTypeName`, hope see the fist item of this issue is done by YOUR PR. 😉
>
> After that, if you are interested in other ones, please be free to move forward!
I compared mysql, postgresql, and oracle, and put some public attributes in Keyword.g4, and others in the corresponding database keyword.g4 file. Some attributes may be in the wrong position, please correct me.
The following is a reference link
[mysql](https://dev.mysql.com/doc/refman/8.0/en/data-types.html)
[postgresql](http://www.postgres.cn/docs/11/datatype.html)
[oracle](https://docs.oracle.com/cd/B28359_01/server.111/b28318/datatype.htm#CNCPT012)
----------------------------------------------------------------
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