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/24 14:59:25 UTC
[GitHub] [shardingsphere] yx9o opened a new pull request, #23086: Add IF NOT EXISTS to CREATE SHADOW RULE.
yx9o opened a new pull request, #23086:
URL: https://github.com/apache/shardingsphere/pull/23086
For #22484 , add IF NOT EXISTS to CREATE SHADOW RULE.
---
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] codecov-commenter commented on pull request #23086: Add IF NOT EXISTS to CREATE SHADOW RULE.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #23086:
URL: https://github.com/apache/shardingsphere/pull/23086#issuecomment-1364545307
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/23086?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 [#23086](https://codecov.io/gh/apache/shardingsphere/pull/23086?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cbf5fc6) into [master](https://codecov.io/gh/apache/shardingsphere/commit/a185a10ad46c4875877901eb7b45949173d3845c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a185a10) will **increase** coverage by `0.01%`.
> The diff coverage is `84.61%`.
```diff
@@ Coverage Diff @@
## master #23086 +/- ##
============================================
+ Coverage 49.95% 49.96% +0.01%
Complexity 2426 2426
============================================
Files 4112 4112
Lines 57397 57405 +8
Branches 9064 9068 +4
============================================
+ Hits 28675 28685 +10
+ Misses 26213 26211 -2
Partials 2509 2509
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/23086?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...sql/parser/core/ShadowDistSQLStatementVisitor.java](https://codecov.io/gh/apache/shardingsphere/pull/23086/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-ZmVhdHVyZXMvc2hhZG93L2Rpc3RzcWwvcGFyc2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zaGFkb3cvZGlzdHNxbC9wYXJzZXIvY29yZS9TaGFkb3dEaXN0U1FMU3RhdGVtZW50VmlzaXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ndler/update/CreateShadowRuleStatementUpdater.java](https://codecov.io/gh/apache/shardingsphere/pull/23086/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-ZmVhdHVyZXMvc2hhZG93L2Rpc3RzcWwvaGFuZGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhZG93L2Rpc3RzcWwvaGFuZGxlci91cGRhdGUvQ3JlYXRlU2hhZG93UnVsZVN0YXRlbWVudFVwZGF0ZXIuamF2YQ==) | `72.97% <91.66%> (+10.11%)` | :arrow_up: |
| [...ingsphere/agent/core/util/AgentReflectionUtil.java](https://codecov.io/gh/apache/shardingsphere/pull/23086/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-YWdlbnQvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvYWdlbnQvY29yZS91dGlsL0FnZW50UmVmbGVjdGlvblV0aWwuamF2YQ==) | `50.00% <0.00%> (-4.55%)` | :arrow_down: |
| [...handler/distsql/ral/hint/enums/HintSourceType.java](https://codecov.io/gh/apache/shardingsphere/pull/23086/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==) | `42.85% <0.00%> (+42.85%)` | :arrow_up: |
: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] yx9o commented on a diff in pull request #23086: Add IF NOT EXISTS to CREATE SHADOW RULE.
Posted by GitBox <gi...@apache.org>.
yx9o commented on code in PR #23086:
URL: https://github.com/apache/shardingsphere/pull/23086#discussion_r1056954344
##########
features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/CreateShadowRuleStatementUpdater.java:
##########
@@ -66,17 +68,23 @@ private void updateTables(final Map<String, ShadowTableConfiguration> currentTab
public void checkSQLStatement(final ShardingSphereDatabase database, final CreateShadowRuleStatement sqlStatement, final ShadowRuleConfiguration currentRuleConfig) {
String databaseName = database.getName();
Collection<ShadowRuleSegment> rules = sqlStatement.getRules();
- checkRuleNames(databaseName, rules, currentRuleConfig);
+ checkRuleNames(databaseName, sqlStatement, currentRuleConfig);
checkResources(database, rules);
- checkAlgorithms(databaseName, sqlStatement.getRules());
- checkAlgorithmType(sqlStatement);
+ checkAlgorithms(databaseName, rules);
+ checkAlgorithmType(rules);
}
- private void checkRuleNames(final String databaseName, final Collection<ShadowRuleSegment> rules, final ShadowRuleConfiguration currentRuleConfig) {
- Collection<String> requireRuleNames = ShadowRuleStatementSupporter.getRuleNames(rules);
+ private void checkRuleNames(final String databaseName, final CreateShadowRuleStatement sqlStatement, final ShadowRuleConfiguration currentRuleConfig) {
+ Collection<String> requireRuleNames = ShadowRuleStatementSupporter.getRuleNames(sqlStatement.getRules());
ShadowRuleStatementChecker.checkAnyDuplicate(requireRuleNames, duplicated -> new DuplicateRuleException(SHADOW, databaseName, duplicated));
- Collection<String> currentRuleName = ShadowRuleStatementSupporter.getRuleNames(currentRuleConfig);
- ShadowRuleStatementChecker.checkAnyDuplicate(requireRuleNames, currentRuleName, identical -> new DuplicateRuleException(SHADOW, databaseName, identical));
+ identicalRuleNames = getIdentical(requireRuleNames, ShadowRuleStatementSupporter.getRuleNames(currentRuleConfig));
+ if (!sqlStatement.isIfNotExists()) {
+ ShardingSpherePreconditions.checkState(identicalRuleNames.isEmpty(), () -> new DuplicateRuleException(SHADOW, databaseName, identicalRuleNames));
+ }
+ }
+
+ private static Collection<String> getIdentical(final Collection<String> require, final Collection<String> current) {
Review Comment:
> Hi @yx9o , can this method be replaced with `retainAll`, do they mean the same?
ok.
--
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 #23086: Add IF NOT EXISTS to CREATE SHADOW RULE.
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #23086:
URL: https://github.com/apache/shardingsphere/pull/23086#discussion_r1056906314
##########
features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/CreateShadowRuleStatementUpdater.java:
##########
@@ -66,17 +68,23 @@ private void updateTables(final Map<String, ShadowTableConfiguration> currentTab
public void checkSQLStatement(final ShardingSphereDatabase database, final CreateShadowRuleStatement sqlStatement, final ShadowRuleConfiguration currentRuleConfig) {
String databaseName = database.getName();
Collection<ShadowRuleSegment> rules = sqlStatement.getRules();
- checkRuleNames(databaseName, rules, currentRuleConfig);
+ checkRuleNames(databaseName, sqlStatement, currentRuleConfig);
checkResources(database, rules);
- checkAlgorithms(databaseName, sqlStatement.getRules());
- checkAlgorithmType(sqlStatement);
+ checkAlgorithms(databaseName, rules);
+ checkAlgorithmType(rules);
}
- private void checkRuleNames(final String databaseName, final Collection<ShadowRuleSegment> rules, final ShadowRuleConfiguration currentRuleConfig) {
- Collection<String> requireRuleNames = ShadowRuleStatementSupporter.getRuleNames(rules);
+ private void checkRuleNames(final String databaseName, final CreateShadowRuleStatement sqlStatement, final ShadowRuleConfiguration currentRuleConfig) {
+ Collection<String> requireRuleNames = ShadowRuleStatementSupporter.getRuleNames(sqlStatement.getRules());
ShadowRuleStatementChecker.checkAnyDuplicate(requireRuleNames, duplicated -> new DuplicateRuleException(SHADOW, databaseName, duplicated));
- Collection<String> currentRuleName = ShadowRuleStatementSupporter.getRuleNames(currentRuleConfig);
- ShadowRuleStatementChecker.checkAnyDuplicate(requireRuleNames, currentRuleName, identical -> new DuplicateRuleException(SHADOW, databaseName, identical));
+ identicalRuleNames = getIdentical(requireRuleNames, ShadowRuleStatementSupporter.getRuleNames(currentRuleConfig));
+ if (!sqlStatement.isIfNotExists()) {
+ ShardingSpherePreconditions.checkState(identicalRuleNames.isEmpty(), () -> new DuplicateRuleException(SHADOW, databaseName, identicalRuleNames));
+ }
+ }
+
+ private static Collection<String> getIdentical(final Collection<String> require, final Collection<String> current) {
Review Comment:
Hi @yx9o , can this method be replaced with `retainAll`, do they mean the same?
--
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 #23086: Add IF NOT EXISTS to CREATE SHADOW RULE.
Posted by GitBox <gi...@apache.org>.
RaigorJiang merged PR #23086:
URL: https://github.com/apache/shardingsphere/pull/23086
--
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 #23086: Add IF NOT EXISTS to CREATE SHADOW RULE.
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #23086:
URL: https://github.com/apache/shardingsphere/pull/23086#discussion_r1056905955
##########
features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/CreateShadowRuleStatementUpdater.java:
##########
@@ -66,17 +68,23 @@ private void updateTables(final Map<String, ShadowTableConfiguration> currentTab
public void checkSQLStatement(final ShardingSphereDatabase database, final CreateShadowRuleStatement sqlStatement, final ShadowRuleConfiguration currentRuleConfig) {
String databaseName = database.getName();
Collection<ShadowRuleSegment> rules = sqlStatement.getRules();
- checkRuleNames(databaseName, rules, currentRuleConfig);
+ checkRuleNames(databaseName, sqlStatement, currentRuleConfig);
checkResources(database, rules);
- checkAlgorithms(databaseName, sqlStatement.getRules());
- checkAlgorithmType(sqlStatement);
+ checkAlgorithms(databaseName, rules);
+ checkAlgorithmType(rules);
}
- private void checkRuleNames(final String databaseName, final Collection<ShadowRuleSegment> rules, final ShadowRuleConfiguration currentRuleConfig) {
- Collection<String> requireRuleNames = ShadowRuleStatementSupporter.getRuleNames(rules);
+ private void checkRuleNames(final String databaseName, final CreateShadowRuleStatement sqlStatement, final ShadowRuleConfiguration currentRuleConfig) {
+ Collection<String> requireRuleNames = ShadowRuleStatementSupporter.getRuleNames(sqlStatement.getRules());
Review Comment:
Does the requireRuleNames mean toBeCreatedRuleNames?
--
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