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 2022/12/25 10:38:57 UTC
[GitHub] [shardingsphere] Qianyi951015 opened a new pull request, #23093: Add `IF NOT EXISTS` for `create sharding table rule`
Qianyi951015 opened a new pull request, #23093:
URL: https://github.com/apache/shardingsphere/pull/23093
The second task of #22844.
Changes proposed in this pull request:
-
---
Before committing this PR, I'm sure that I have checked the following options:
- [x] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [x] I have self-reviewed the commit code.
- [ ] I have (or in comment I request) added corresponding labels for the pull request.
- [x] 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.
- [x] 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] RaigorJiang commented on a diff in pull request #23093: Add `IF NOT EXISTS` for `create sharding table rule`
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #23093:
URL: https://github.com/apache/shardingsphere/pull/23093#discussion_r1056975975
##########
features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/update/CreateShardingTableRuleStatementUpdater.java:
##########
@@ -41,6 +47,16 @@ public ShardingRuleConfiguration buildToBeCreatedRuleConfiguration(final CreateS
@Override
public void updateCurrentRuleConfiguration(final ShardingRuleConfiguration currentRuleConfig, final ShardingRuleConfiguration toBeCreatedRuleConfig) {
+ if (ifNotExists) {
+ Collection<String> currentTables = new LinkedList<>();
+ currentRuleConfig.getTables().forEach(each -> currentTables.add(each.getLogicTable()));
+ currentRuleConfig.getAutoTables().forEach(each -> currentTables.add(each.getLogicTable()));
+ toBeCreatedRuleConfig.getTables().removeIf(shardingTableRuleConfiguration -> currentTables.contains(shardingTableRuleConfiguration.getLogicTable()));
Review Comment:
`shardingTableRuleConfiguration` can be replaced with `each`
--
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] RaigorJiang commented on a diff in pull request #23093: Add `IF NOT EXISTS` for `create sharding table rule`
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #23093:
URL: https://github.com/apache/shardingsphere/pull/23093#discussion_r1056975707
##########
features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -89,10 +89,12 @@ public final class ShardingTableRuleStatementChecker {
*
* @param database database
* @param rules rules
+ * @param ifNotExists ifNotExists statement if exists
Review Comment:
`ifNotExists` is not a statement, is it a clause?
--
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] RaigorJiang merged pull request #23093: Add `IF NOT EXISTS` for `create sharding table rule`
Posted by GitBox <gi...@apache.org>.
RaigorJiang merged PR #23093:
URL: https://github.com/apache/shardingsphere/pull/23093
--
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 #23093: Add `IF NOT EXISTS` for `create sharding table rule`
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #23093:
URL: https://github.com/apache/shardingsphere/pull/23093#issuecomment-1364664627
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/23093?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 [#23093](https://codecov.io/gh/apache/shardingsphere/pull/23093?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (18e93e4) into [master](https://codecov.io/gh/apache/shardingsphere/commit/eb047d26f36a2c85d89d8e8026c5f7bdecda5dff?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (eb047d2) will **increase** coverage by `0.00%`.
> The diff coverage is `88.88%`.
```diff
@@ Coverage Diff @@
## master #23093 +/- ##
=========================================
Coverage 49.96% 49.96%
- Complexity 2426 2427 +1
=========================================
Files 4110 4110
Lines 57395 57405 +10
Branches 9063 9067 +4
=========================================
+ Hits 28679 28685 +6
- Misses 26206 26209 +3
- Partials 2510 2511 +1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/23093?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...l/parser/core/ShardingDistSQLStatementVisitor.java](https://codecov.io/gh/apache/shardingsphere/pull/23093/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmVhdHVyZXMvc2hhcmRpbmcvZGlzdHNxbC9wYXJzZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NoYXJkaW5nL2Rpc3RzcWwvcGFyc2VyL2NvcmUvU2hhcmRpbmdEaXN0U1FMU3RhdGVtZW50VmlzaXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...pdate/CreateShardingTableRuleStatementUpdater.java](https://codecov.io/gh/apache/shardingsphere/pull/23093/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmVhdHVyZXMvc2hhcmRpbmcvZGlzdHNxbC9oYW5kbGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zaGFyZGluZy9kaXN0c3FsL2hhbmRsZXIvdXBkYXRlL0NyZWF0ZVNoYXJkaW5nVGFibGVSdWxlU3RhdGVtZW50VXBkYXRlci5qYXZh) | `85.71% <90.00%> (+2.38%)` | :arrow_up: |
| [...ler/checker/ShardingTableRuleStatementChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/23093/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmVhdHVyZXMvc2hhcmRpbmcvZGlzdHNxbC9oYW5kbGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zaGFyZGluZy9kaXN0c3FsL2hhbmRsZXIvY2hlY2tlci9TaGFyZGluZ1RhYmxlUnVsZVN0YXRlbWVudENoZWNrZXIuamF2YQ==) | `83.47% <100.00%> (+0.06%)` | :arrow_up: |
| [...handler/distsql/ral/hint/enums/HintSourceType.java](https://codecov.io/gh/apache/shardingsphere/pull/23093/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJveHkvYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC9oYW5kbGVyL2Rpc3RzcWwvcmFsL2hpbnQvZW51bXMvSGludFNvdXJjZVR5cGUuamF2YQ==) | `0.00% <0.00%> (-42.86%)` | :arrow_down: |
: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] RaigorJiang commented on a diff in pull request #23093: Add `IF NOT EXISTS` for `create sharding table rule`
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #23093:
URL: https://github.com/apache/shardingsphere/pull/23093#discussion_r1056976137
##########
features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -205,14 +208,17 @@ private static boolean isValidDataNode(final String dataNodeStr) {
return dataNodeStr.contains(DELIMITER) && 2 == Splitter.on(DELIMITER).omitEmptyStrings().splitToList(dataNodeStr).size();
}
- private static void checkTables(final String databaseName, final Collection<AbstractTableRuleSegment> rules, final ShardingRuleConfiguration currentRuleConfig, final boolean isCreate) {
+ private static void checkTables(final String databaseName, final Collection<AbstractTableRuleSegment> rules, final ShardingRuleConfiguration currentRuleConfig, final boolean isCreate,
+ final boolean ifNotExists) {
Collection<String> requiredTables = rules.stream().map(AbstractTableRuleSegment::getLogicTable).collect(Collectors.toList());
Collection<String> duplicatedTables = getDuplicate(requiredTables);
ShardingSpherePreconditions.checkState(duplicatedTables.isEmpty(), () -> new DuplicateRuleException("sharding", databaseName, duplicatedTables));
Collection<String> currentShardingTables = null == currentRuleConfig ? Collections.emptyList() : getCurrentShardingTables(currentRuleConfig);
if (isCreate) {
- Collection<String> identical = getIdentical(requiredTables, currentShardingTables);
- ShardingSpherePreconditions.checkState(identical.isEmpty(), () -> new DuplicateRuleException("sharding", databaseName, identical));
+ if (!ifNotExists) {
+ Collection<String> identical = getIdentical(requiredTables, currentShardingTables);
Review Comment:
Does the `identical` means `duplicatedRules`?
--
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