You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "azexcy (via GitHub)" <gi...@apache.org> on 2023/02/06 10:22:09 UTC
[GitHub] [shardingsphere] azexcy opened a new pull request, #24026: Fix Migration not support PostgreSQL json type
azexcy opened a new pull request, #24026:
URL: https://github.com/apache/shardingsphere/pull/24026
Fixes #24023.
Changes proposed in this pull request:
- Fix Migration not support PostgreSQL json type
- Pipeline E2E update `t_json` column value
---
Before committing this PR, I'm sure that I have checked the following options:
- [ ] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [ ] I have self-reviewed the commit code.
- [ ] I have (or in comment I request) added corresponding labels for the pull request.
- [ ] I have passed maven check locally : `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
- [ ] I have made corresponding changes to the documentation.
- [ ] I have added corresponding unit tests for my changes.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] sandynz commented on a diff in pull request #24026: Fix Migration not support PostgreSQL json type
Posted by "sandynz (via GitHub)" <gi...@apache.org>.
sandynz commented on code in PR #24026:
URL: https://github.com/apache/shardingsphere/pull/24026#discussion_r1098594560
##########
test/e2e/pipeline/src/test/java/org/apache/shardingsphere/test/e2e/data/pipeline/framework/helper/PipelineCaseHelper.java:
##########
@@ -102,8 +102,21 @@ private static String generateString(final int strLength) {
return RandomStringUtils.randomAlphabetic(strLength);
}
- private static String generateJsonString(final int strLength) {
- return String.format("{\"test\":\"%s\"}", generateString(strLength));
+ /**
+ * Generate json string.
+ *
+ * @param useUnicodeCharacter use unicode character
+ * @param strLength string length
+ * @return json string
+ */
+ public static String generateJsonString(final int strLength, final boolean useUnicodeCharacter) {
Review Comment:
`strLength` could be `length`
##########
test/e2e/pipeline/src/test/java/org/apache/shardingsphere/test/e2e/data/pipeline/cases/base/PipelineBaseE2EIT.java:
##########
@@ -201,9 +201,13 @@ protected void addResource(final String distSQL) throws SQLException {
}
protected String appendExtraParam(final String jdbcUrl) {
- return DatabaseTypeUtil.isMySQL(getDatabaseType())
- ? new JdbcUrlAppender().appendQueryProperties(jdbcUrl, PropertiesBuilder.build(new Property("rewriteBatchedStatements", Boolean.TRUE.toString())))
- : jdbcUrl;
+ if (DatabaseTypeUtil.isMySQL(getDatabaseType())) {
+ return new JdbcUrlAppender().appendQueryProperties(jdbcUrl, PropertiesBuilder.build(new Property("rewriteBatchedStatements", Boolean.TRUE.toString())));
+ }
+ if (DatabaseTypeUtil.isPostgreSQL(getDatabaseType()) || DatabaseTypeUtil.isOpenGauss(getDatabaseType())) {
+ return new JdbcUrlAppender().appendQueryProperties(jdbcUrl, PropertiesBuilder.build(new Property("stringtype", "unspecified")));
+ }
+ return jdbcUrl;
Review Comment:
`jdbcUrl` should be `result`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] sandynz commented on pull request #24026: Fix Migration not support PostgreSQL json type
Posted by "sandynz (via GitHub)" <gi...@apache.org>.
sandynz commented on PR #24026:
URL: https://github.com/apache/shardingsphere/pull/24026#issuecomment-1420694487
Unicode character support could be done in next PR
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] azexcy commented on pull request #24026: Fix Migration not support PostgreSQL json type
Posted by "azexcy (via GitHub)" <gi...@apache.org>.
azexcy commented on PR #24026:
URL: https://github.com/apache/shardingsphere/pull/24026#issuecomment-1420061910
reference documents
```
https://jdbc.postgresql.org/documentation/use/
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] sandynz merged pull request #24026: Fix Migration not support PostgreSQL json type
Posted by "sandynz (via GitHub)" <gi...@apache.org>.
sandynz merged PR #24026:
URL: https://github.com/apache/shardingsphere/pull/24026
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] azexcy commented on pull request #24026: Fix Migration not support PostgreSQL json type
Posted by "azexcy (via GitHub)" <gi...@apache.org>.
azexcy commented on PR #24026:
URL: https://github.com/apache/shardingsphere/pull/24026#issuecomment-1420245285
Data consistency checks will fail when only unique keys are found, fixed at this pr
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] sandynz commented on a diff in pull request #24026: Fix Migration not support PostgreSQL json type
Posted by "sandynz (via GitHub)" <gi...@apache.org>.
sandynz commented on code in PR #24026:
URL: https://github.com/apache/shardingsphere/pull/24026#discussion_r1098105010
##########
test/e2e/pipeline/src/test/java/org/apache/shardingsphere/test/e2e/data/pipeline/cases/task/PostgreSQLIncrementTask.java:
##########
@@ -73,8 +73,8 @@ public void run() {
private Object insertOrder() {
ThreadLocalRandom random = ThreadLocalRandom.current();
String status = 0 == random.nextInt() % 2 ? null : "中文测试";
- Object[] orderInsertDate = new Object[]{KEY_GENERATE_ALGORITHM.generateKey(), random.nextInt(0, 6), status};
- String insertSQL = String.format("INSERT INTO %s (order_id,user_id,status) VALUES (?, ?, ?)", getTableNameWithSchema(orderTableName));
+ Object[] orderInsertDate = new Object[]{KEY_GENERATE_ALGORITHM.generateKey(), random.nextInt(0, 6), status, PipelineCaseHelper.generateJsonString(2)};
+ String insertSQL = String.format("INSERT INTO %s (order_id,user_id,status,t_json) VALUES (?, ?, ?, ?)", getTableNameWithSchema(orderTableName));
Review Comment:
It's better to support `jsonb` at this time
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] codecov-commenter commented on pull request #24026: Fix Migration not support PostgreSQL json type
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24026:
URL: https://github.com/apache/shardingsphere/pull/24026#issuecomment-1418964855
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/24026?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#24026](https://codecov.io/gh/apache/shardingsphere/pull/24026?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5478850) into [master](https://codecov.io/gh/apache/shardingsphere/commit/ade41696f7cc16340caf737a8b5889f8a288f2ce?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ade4169) will **increase** coverage by `0.00%`.
> The diff coverage is `4.76%`.
> :exclamation: Current head 5478850 differs from pull request most recent head e994b09. Consider uploading reports for the commit e994b09 to get more accurate results
```diff
@@ Coverage Diff @@
## master #24026 +/- ##
=========================================
Coverage 50.05% 50.05%
Complexity 1564 1564
=========================================
Files 3244 3246 +2
Lines 53273 53272 -1
Branches 9801 9802 +1
=========================================
+ Hits 26665 26666 +1
+ Misses 24257 24255 -2
Partials 2351 2351
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/24026?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ding/metadata/ShardingSchemaMetaDataDecorator.java](https://codecov.io/gh/apache/shardingsphere/pull/24026?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmVhdHVyZXMvc2hhcmRpbmcvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvbWV0YWRhdGEvU2hhcmRpbmdTY2hlbWFNZXRhRGF0YURlY29yYXRvci5qYXZh) | `65.62% <0.00%> (+24.44%)` | :arrow_up: |
| [...ng/metadata/reviser/ShardingConstraintReviser.java](https://codecov.io/gh/apache/shardingsphere/pull/24026?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmVhdHVyZXMvc2hhcmRpbmcvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvbWV0YWRhdGEvcmV2aXNlci9TaGFyZGluZ0NvbnN0cmFpbnRSZXZpc2VyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...harding/metadata/reviser/ShardingIndexReviser.java](https://codecov.io/gh/apache/shardingsphere/pull/24026?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmVhdHVyZXMvc2hhcmRpbmcvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvbWV0YWRhdGEvcmV2aXNlci9TaGFyZGluZ0luZGV4UmV2aXNlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...source/PostgreSQLJdbcQueryPropertiesExtension.java](https://codecov.io/gh/apache/shardingsphere/pull/24026?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a2VybmVsL2RhdGEtcGlwZWxpbmUvZGlhbGVjdC9wb3N0Z3Jlc3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL3Bvc3RncmVzcWwvZGF0YXNvdXJjZS9Qb3N0Z3JlU1FMSmRiY1F1ZXJ5UHJvcGVydGllc0V4dGVuc2lvbi5qYXZh) | `100.00% <100.00%> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] sandynz commented on a diff in pull request #24026: Fix Migration not support PostgreSQL json type
Posted by "sandynz (via GitHub)" <gi...@apache.org>.
sandynz commented on code in PR #24026:
URL: https://github.com/apache/shardingsphere/pull/24026#discussion_r1098116551
##########
test/e2e/pipeline/src/test/java/org/apache/shardingsphere/test/e2e/data/pipeline/cases/task/PostgreSQLIncrementTask.java:
##########
@@ -73,8 +73,9 @@ public void run() {
private Object insertOrder() {
ThreadLocalRandom random = ThreadLocalRandom.current();
String status = 0 == random.nextInt() % 2 ? null : "中文测试";
- Object[] orderInsertDate = new Object[]{KEY_GENERATE_ALGORITHM.generateKey(), random.nextInt(0, 6), status, PipelineCaseHelper.generateJsonString(2)};
- String insertSQL = String.format("INSERT INTO %s (order_id,user_id,status,t_json) VALUES (?, ?, ?, ?)", getTableNameWithSchema(orderTableName));
+ String jsonString = PipelineCaseHelper.generateJsonString(2);
+ Object[] orderInsertDate = new Object[]{KEY_GENERATE_ALGORITHM.generateKey(), random.nextInt(0, 6), status, jsonString, jsonString.getBytes()};
Review Comment:
1, `jsonString.getBytes()`, could we support Unicode character?
2, It's better to make `PipelineCaseHelper.generateJsonString(2)` could generate Unicode characters.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] azexcy commented on a diff in pull request #24026: Fix Migration not support PostgreSQL json type
Posted by "azexcy (via GitHub)" <gi...@apache.org>.
azexcy commented on code in PR #24026:
URL: https://github.com/apache/shardingsphere/pull/24026#discussion_r1098143170
##########
test/e2e/pipeline/src/test/java/org/apache/shardingsphere/test/e2e/data/pipeline/cases/task/PostgreSQLIncrementTask.java:
##########
@@ -73,8 +73,9 @@ public void run() {
private Object insertOrder() {
ThreadLocalRandom random = ThreadLocalRandom.current();
String status = 0 == random.nextInt() % 2 ? null : "中文测试";
- Object[] orderInsertDate = new Object[]{KEY_GENERATE_ALGORITHM.generateKey(), random.nextInt(0, 6), status, PipelineCaseHelper.generateJsonString(2)};
- String insertSQL = String.format("INSERT INTO %s (order_id,user_id,status,t_json) VALUES (?, ?, ?, ?)", getTableNameWithSchema(orderTableName));
+ String jsonString = PipelineCaseHelper.generateJsonString(2);
+ Object[] orderInsertDate = new Object[]{KEY_GENERATE_ALGORITHM.generateKey(), random.nextInt(0, 6), status, jsonString, jsonString.getBytes()};
Review Comment:
I find data consistency check failed, i need takes a little time to check
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] azexcy commented on a diff in pull request #24026: Fix Migration not support PostgreSQL json type
Posted by "azexcy (via GitHub)" <gi...@apache.org>.
azexcy commented on code in PR #24026:
URL: https://github.com/apache/shardingsphere/pull/24026#discussion_r1098143170
##########
test/e2e/pipeline/src/test/java/org/apache/shardingsphere/test/e2e/data/pipeline/cases/task/PostgreSQLIncrementTask.java:
##########
@@ -73,8 +73,9 @@ public void run() {
private Object insertOrder() {
ThreadLocalRandom random = ThreadLocalRandom.current();
String status = 0 == random.nextInt() % 2 ? null : "中文测试";
- Object[] orderInsertDate = new Object[]{KEY_GENERATE_ALGORITHM.generateKey(), random.nextInt(0, 6), status, PipelineCaseHelper.generateJsonString(2)};
- String insertSQL = String.format("INSERT INTO %s (order_id,user_id,status,t_json) VALUES (?, ?, ?, ?)", getTableNameWithSchema(orderTableName));
+ String jsonString = PipelineCaseHelper.generateJsonString(2);
+ Object[] orderInsertDate = new Object[]{KEY_GENERATE_ALGORITHM.generateKey(), random.nextInt(0, 6), status, jsonString, jsonString.getBytes()};
Review Comment:
I find data consistency check failed, i need takes a little time to check
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org