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/09/04 15:57:51 UTC
[GitHub] [shardingsphere] sunkai-cai opened a new pull request, #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
sunkai-cai opened a new pull request, #20777:
URL: https://github.com/apache/shardingsphere/pull/20777
Fixes #20378.
Changes proposed in this pull request:
- It must not use the Auto Sharding Algorithm In the tableRuleSegment
--
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 #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r964417503
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -278,24 +278,34 @@ private static void checkTableRule(final String databaseName, final ShardingRule
}
private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ Map<String, AlgorithmConfiguration> currentAlgorithms = null == currentRuleConfig ? new HashMap<>() : currentRuleConfig.getShardingAlgorithms();
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
- .map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
+ .map(each -> null != each.getShardingAlgorithmName() ? each.getShardingAlgorithmName() : (null != each.getAlgorithmSegment() ? each.getAlgorithmSegment().getName() : ""))
Review Comment:
conditional operator( ? : ) nested use is forbidden.
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -278,24 +278,34 @@ private static void checkTableRule(final String databaseName, final ShardingRule
}
private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ Map<String, AlgorithmConfiguration> currentAlgorithms = null == currentRuleConfig ? new HashMap<>() : currentRuleConfig.getShardingAlgorithms();
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
- .map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
+ .map(each -> null != each.getShardingAlgorithmName() ? each.getShardingAlgorithmName() : (null != each.getAlgorithmSegment() ? each.getAlgorithmSegment().getName() : ""))
Review Comment:
https://shardingsphere.apache.org/community/en/involved/conduct/code/
--
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 #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r975573335
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -273,29 +273,50 @@ private static void checkTableRule(final String databaseName, final ShardingRule
Optional<ShardingStrategySegment> anyTableRule = tableRules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).findAny();
if (anyTableRule.isPresent()) {
- checkStrategy(databaseName, currentRuleConfig, tableRules);
+ checkStrategy(databaseName, tableRules);
+ checkAlgorithm(databaseName, currentRuleConfig, tableRules);
}
}
- private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ private static void checkStrategy(final String databaseName, final Collection<TableRuleSegment> rules) throws DistSQLException {
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
- .flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
+ .flatMap(Collection::stream).filter(Objects::nonNull).filter(ShardingTableRuleStatementChecker::isInvalidStrategy)
.map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
DistSQLException.predictionThrow(invalidAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException(databaseName, invalidAlgorithms));
}
- private static boolean isInvalidStrategy(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isInvalidStrategy(final ShardingStrategySegment shardingStrategySegment) {
return !ShardingStrategyType.contain(shardingStrategySegment.getType())
- || !ShardingStrategyType.getValueOf(shardingStrategySegment.getType()).isValid(shardingStrategySegment.getShardingColumn())
- || !isAlgorithmExists(currentAlgorithms, shardingStrategySegment);
+ || !ShardingStrategyType.getValueOf(shardingStrategySegment.getType()).isValid(shardingStrategySegment.getShardingColumn());
}
- private static boolean isAlgorithmExists(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
- if (null == shardingStrategySegment.getShardingAlgorithmName() && null != shardingStrategySegment.getAlgorithmSegment()) {
- return true;
+ private static void checkAlgorithm(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
+ Map<String, AlgorithmConfiguration> currentAlgorithms = Optional.ofNullable(currentRuleConfig).map(ShardingRuleConfiguration::getShardingAlgorithms).orElse(Collections.emptyMap());
+ Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment())).flatMap(Collection::stream)
+ .filter(Objects::nonNull).map(each -> getInvalidAlgorithmName(currentAlgorithms, each)).filter(Objects::nonNull).collect(Collectors.toList());
+ DistSQLException.predictionThrow(invalidAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException(databaseName, invalidAlgorithms));
+ }
+
+ private static String getInvalidAlgorithmName(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ ShardingAlgorithm shardingAlgorithm;
+ String shardingAlgorithmName;
+ if (null == shardingStrategySegment.getShardingAlgorithmName() && null == shardingStrategySegment.getAlgorithmSegment()) {
+ return "ShardingAlgorithm is not fount";
+ } else if (null != shardingStrategySegment.getAlgorithmSegment()) {
+ shardingAlgorithmName = shardingStrategySegment.getAlgorithmSegment().getName();
Review Comment:
The result of `shardingStrategySegment.getAlgorithmSegment().getName()` is actually the algorithm type name, like `INLINE`, but not the algorithm name.
--
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 #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r975571929
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -273,29 +273,50 @@ private static void checkTableRule(final String databaseName, final ShardingRule
Optional<ShardingStrategySegment> anyTableRule = tableRules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).findAny();
if (anyTableRule.isPresent()) {
- checkStrategy(databaseName, currentRuleConfig, tableRules);
+ checkStrategy(databaseName, tableRules);
+ checkAlgorithm(databaseName, currentRuleConfig, tableRules);
}
}
- private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ private static void checkStrategy(final String databaseName, final Collection<TableRuleSegment> rules) throws DistSQLException {
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
- .flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
+ .flatMap(Collection::stream).filter(Objects::nonNull).filter(ShardingTableRuleStatementChecker::isInvalidStrategy)
.map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
DistSQLException.predictionThrow(invalidAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException(databaseName, invalidAlgorithms));
}
- private static boolean isInvalidStrategy(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isInvalidStrategy(final ShardingStrategySegment shardingStrategySegment) {
return !ShardingStrategyType.contain(shardingStrategySegment.getType())
- || !ShardingStrategyType.getValueOf(shardingStrategySegment.getType()).isValid(shardingStrategySegment.getShardingColumn())
- || !isAlgorithmExists(currentAlgorithms, shardingStrategySegment);
+ || !ShardingStrategyType.getValueOf(shardingStrategySegment.getType()).isValid(shardingStrategySegment.getShardingColumn());
}
- private static boolean isAlgorithmExists(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
- if (null == shardingStrategySegment.getShardingAlgorithmName() && null != shardingStrategySegment.getAlgorithmSegment()) {
- return true;
+ private static void checkAlgorithm(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
+ Map<String, AlgorithmConfiguration> currentAlgorithms = Optional.ofNullable(currentRuleConfig).map(ShardingRuleConfiguration::getShardingAlgorithms).orElse(Collections.emptyMap());
+ Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment())).flatMap(Collection::stream)
+ .filter(Objects::nonNull).map(each -> getInvalidAlgorithmName(currentAlgorithms, each)).filter(Objects::nonNull).collect(Collectors.toList());
+ DistSQLException.predictionThrow(invalidAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException(databaseName, invalidAlgorithms));
+ }
+
+ private static String getInvalidAlgorithmName(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ ShardingAlgorithm shardingAlgorithm;
+ String shardingAlgorithmName;
+ if (null == shardingStrategySegment.getShardingAlgorithmName() && null == shardingStrategySegment.getAlgorithmSegment()) {
+ return "ShardingAlgorithm is not fount";
Review Comment:
This line does not match the method name.
--
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] sunkai-cai commented on a diff in pull request #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
sunkai-cai commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r976636220
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -273,29 +273,50 @@ private static void checkTableRule(final String databaseName, final ShardingRule
Optional<ShardingStrategySegment> anyTableRule = tableRules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).findAny();
if (anyTableRule.isPresent()) {
- checkStrategy(databaseName, currentRuleConfig, tableRules);
+ checkStrategy(databaseName, tableRules);
+ checkAlgorithm(databaseName, currentRuleConfig, tableRules);
}
}
- private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ private static void checkStrategy(final String databaseName, final Collection<TableRuleSegment> rules) throws DistSQLException {
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
- .flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
+ .flatMap(Collection::stream).filter(Objects::nonNull).filter(ShardingTableRuleStatementChecker::isInvalidStrategy)
.map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
DistSQLException.predictionThrow(invalidAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException(databaseName, invalidAlgorithms));
}
- private static boolean isInvalidStrategy(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isInvalidStrategy(final ShardingStrategySegment shardingStrategySegment) {
return !ShardingStrategyType.contain(shardingStrategySegment.getType())
- || !ShardingStrategyType.getValueOf(shardingStrategySegment.getType()).isValid(shardingStrategySegment.getShardingColumn())
- || !isAlgorithmExists(currentAlgorithms, shardingStrategySegment);
+ || !ShardingStrategyType.getValueOf(shardingStrategySegment.getType()).isValid(shardingStrategySegment.getShardingColumn());
}
- private static boolean isAlgorithmExists(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
- if (null == shardingStrategySegment.getShardingAlgorithmName() && null != shardingStrategySegment.getAlgorithmSegment()) {
- return true;
+ private static void checkAlgorithm(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
+ Map<String, AlgorithmConfiguration> currentAlgorithms = Optional.ofNullable(currentRuleConfig).map(ShardingRuleConfiguration::getShardingAlgorithms).orElse(Collections.emptyMap());
+ Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment())).flatMap(Collection::stream)
+ .filter(Objects::nonNull).map(each -> getInvalidAlgorithmName(currentAlgorithms, each)).filter(Objects::nonNull).collect(Collectors.toList());
+ DistSQLException.predictionThrow(invalidAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException(databaseName, invalidAlgorithms));
+ }
+
+ private static String getInvalidAlgorithmName(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ ShardingAlgorithm shardingAlgorithm;
+ String shardingAlgorithmName;
+ if (null == shardingStrategySegment.getShardingAlgorithmName() && null == shardingStrategySegment.getAlgorithmSegment()) {
+ return "ShardingAlgorithm is not fount";
Review Comment:
I change `getInvalidAlgorithmName` to `getInvalidAlgorithmError`, And make it retrun ErrorMessage.
--
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 pull request #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#issuecomment-1288128343
Hi @sunkai-cai
This PR has not been active for a long time, and you may have to deal with many conflicts, I will close it 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.
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 #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r965618851
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -278,24 +278,34 @@ private static void checkTableRule(final String databaseName, final ShardingRule
}
private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ Map<String, AlgorithmConfiguration> currentAlgorithms = Optional.ofNullable(currentRuleConfig).map(ShardingRuleConfiguration::getShardingAlgorithms).orElse(Collections.emptyMap());
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
- .map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
+ .map(each -> Optional.ofNullable(each.getShardingAlgorithmName()).orElse(Optional.ofNullable(each.getAlgorithmSegment()).map(AlgorithmSegment::getName).orElse("unknown")))
Review Comment:
"unknown" is not acceptable information, we should have more discussion, I'll mark it as draft 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.
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 #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r964413609
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -278,24 +278,34 @@ private static void checkTableRule(final String databaseName, final ShardingRule
}
private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ Map<String, AlgorithmConfiguration> currentAlgorithms = null == currentRuleConfig ? new HashMap<>() : currentRuleConfig.getShardingAlgorithms();
Review Comment:
Can `new HashMap<>` be replaced with `Collections.emptyMap()`?
--
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] sunkai-cai commented on a diff in pull request #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
sunkai-cai commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r964866110
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -278,24 +278,34 @@ private static void checkTableRule(final String databaseName, final ShardingRule
}
private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ Map<String, AlgorithmConfiguration> currentAlgorithms = null == currentRuleConfig ? new HashMap<>() : currentRuleConfig.getShardingAlgorithms();
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
- .map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
+ .map(each -> null != each.getShardingAlgorithmName() ? each.getShardingAlgorithmName() : (null != each.getAlgorithmSegment() ? each.getAlgorithmSegment().getName() : ""))
+ .collect(Collectors.toList());
DistSQLException.predictionThrow(invalidAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException(databaseName, invalidAlgorithms));
}
- private static boolean isInvalidStrategy(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isInvalidStrategy(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
return !ShardingStrategyType.contain(shardingStrategySegment.getType())
|| !ShardingStrategyType.getValueOf(shardingStrategySegment.getType()).isValid(shardingStrategySegment.getShardingColumn())
- || !isAlgorithmExists(currentAlgorithms, shardingStrategySegment);
+ || !isStandardAlgorithmExists(currentAlgorithms, shardingStrategySegment);
}
- private static boolean isAlgorithmExists(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isStandardAlgorithmExists(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
Review Comment:
done.
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -278,24 +278,34 @@ private static void checkTableRule(final String databaseName, final ShardingRule
}
private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ Map<String, AlgorithmConfiguration> currentAlgorithms = null == currentRuleConfig ? new HashMap<>() : currentRuleConfig.getShardingAlgorithms();
Review Comment:
done.
--
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] TeslaCN commented on a diff in pull request #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r962438140
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -278,24 +278,35 @@ private static void checkTableRule(final String databaseName, final ShardingRule
}
private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ Map<String, AlgorithmConfiguration> currentAlgorithms = null == currentRuleConfig ? new HashMap<>() : currentRuleConfig.getShardingAlgorithms();
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
- .map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
+ .map(each -> null != each.getShardingAlgorithmName() ? each.getShardingAlgorithmName() : (null != each.getAlgorithmSegment() ? each.getAlgorithmSegment().getName() : ""))
+ .collect(Collectors.toList());
DistSQLException.predictionThrow(invalidAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException(databaseName, invalidAlgorithms));
}
- private static boolean isInvalidStrategy(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isInvalidStrategy(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
return !ShardingStrategyType.contain(shardingStrategySegment.getType())
|| !ShardingStrategyType.getValueOf(shardingStrategySegment.getType()).isValid(shardingStrategySegment.getShardingColumn())
- || !isAlgorithmExists(currentAlgorithms, shardingStrategySegment);
+ || !isStandardAlgorithmExists(currentAlgorithms, shardingStrategySegment);
}
- private static boolean isAlgorithmExists(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isStandardAlgorithmExists(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ ShardingAlgorithm shardingAlgorithm;
if (null == shardingStrategySegment.getShardingAlgorithmName() && null != shardingStrategySegment.getAlgorithmSegment()) {
- return true;
+ shardingAlgorithm = ShardingAlgorithmFactory.newInstance(new AlgorithmConfiguration(
+ shardingStrategySegment.getAlgorithmSegment().getName(),
+ shardingStrategySegment.getAlgorithmSegment().getProps()));
+ } else {
+ AlgorithmConfiguration algorithmConfiguration = currentAlgorithms.get(shardingStrategySegment.getShardingAlgorithmName());
+ if (null == algorithmConfiguration) {
+ return false;
+ }
+ shardingAlgorithm = ShardingAlgorithmFactory.newInstance(algorithmConfiguration);
}
- return currentAlgorithms.contains(shardingStrategySegment.getShardingAlgorithmName());
+
Review Comment:
Please remove the redundant blank line here.
--
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] TeslaCN commented on a diff in pull request #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r962437923
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -84,8 +84,8 @@ public final class ShardingTableRuleStatementChecker {
/**
* Check create sharing table rule statement.
*
- * @param database database
Review Comment:
The changes to javadoc could be reverted.
--
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 #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r964413011
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -278,24 +278,34 @@ private static void checkTableRule(final String databaseName, final ShardingRule
}
private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ Map<String, AlgorithmConfiguration> currentAlgorithms = null == currentRuleConfig ? new HashMap<>() : currentRuleConfig.getShardingAlgorithms();
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
- .map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
+ .map(each -> null != each.getShardingAlgorithmName() ? each.getShardingAlgorithmName() : (null != each.getAlgorithmSegment() ? each.getAlgorithmSegment().getName() : ""))
+ .collect(Collectors.toList());
DistSQLException.predictionThrow(invalidAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException(databaseName, invalidAlgorithms));
}
- private static boolean isInvalidStrategy(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isInvalidStrategy(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
return !ShardingStrategyType.contain(shardingStrategySegment.getType())
|| !ShardingStrategyType.getValueOf(shardingStrategySegment.getType()).isValid(shardingStrategySegment.getShardingColumn())
- || !isAlgorithmExists(currentAlgorithms, shardingStrategySegment);
+ || !isStandardAlgorithmExists(currentAlgorithms, shardingStrategySegment);
}
- private static boolean isAlgorithmExists(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isStandardAlgorithmExists(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
Review Comment:
In fact, there are not only `standard`, but also `complex` and `hint`, so this method renaming is not appropriate.
--
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] sunkai-cai commented on a diff in pull request #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
sunkai-cai commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r964866931
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -278,24 +278,34 @@ private static void checkTableRule(final String databaseName, final ShardingRule
}
private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ Map<String, AlgorithmConfiguration> currentAlgorithms = null == currentRuleConfig ? new HashMap<>() : currentRuleConfig.getShardingAlgorithms();
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
- .map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
+ .map(each -> null != each.getShardingAlgorithmName() ? each.getShardingAlgorithmName() : (null != each.getAlgorithmSegment() ? each.getAlgorithmSegment().getName() : ""))
Review Comment:
thanks the tips. it is done.
--
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 closed pull request #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
RaigorJiang closed pull request #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
URL: https://github.com/apache/shardingsphere/pull/20777
--
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] strongduanmu commented on pull request #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#issuecomment-1275443136
Hi @sunkai-cai, can you solve code conflict?
--
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 #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#issuecomment-1236378926
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/20777?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 [#20777](https://codecov.io/gh/apache/shardingsphere/pull/20777?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c53a336) into [master](https://codecov.io/gh/apache/shardingsphere/commit/75b2452367267dd39e63c1752576d135cbcd1f20?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (75b2452) will **increase** coverage by `0.06%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #20777 +/- ##
============================================
+ Coverage 61.25% 61.32% +0.06%
- Complexity 2419 2426 +7
============================================
Files 3982 3983 +1
Lines 55229 55238 +9
Branches 9373 9375 +2
============================================
+ Hits 33833 33874 +41
+ Misses 18514 18481 -33
- Partials 2882 2883 +1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/20777?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ler/checker/ShardingTableRuleStatementChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/20777/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctZGlzdHNxbC9zaGFyZGluZ3NwaGVyZS1zaGFyZGluZy1kaXN0c3FsLWhhbmRsZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NoYXJkaW5nL2Rpc3RzcWwvaGFuZGxlci9jaGVja2VyL1NoYXJkaW5nVGFibGVSdWxlU3RhdGVtZW50Q2hlY2tlci5qYXZh) | `85.29% <100.00%> (+0.87%)` | :arrow_up: |
| [...ine/type/unicast/ShardingUnicastRoutingEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/20777/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvcm91dGUvZW5naW5lL3R5cGUvdW5pY2FzdC9TaGFyZGluZ1VuaWNhc3RSb3V0aW5nRW5naW5lLmphdmE=) | `95.45% <0.00%> (ø)` | |
| [...ne/scenario/migration/MigrationJobItemContext.java](https://codecov.io/gh/apache/shardingsphere/pull/20777/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL3NjZW5hcmlvL21pZ3JhdGlvbi9NaWdyYXRpb25Kb2JJdGVtQ29udGV4dC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...e/ingest/dumper/DefaultInventoryDumperCreator.java](https://codecov.io/gh/apache/shardingsphere/pull/20777/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvaW5nZXN0L2R1bXBlci9EZWZhdWx0SW52ZW50b3J5RHVtcGVyQ3JlYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ta/pipeline/mysql/MySqlInventoryDumperCreator.java](https://codecov.io/gh/apache/shardingsphere/pull/20777/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1kaWFsZWN0L3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUtbXlzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvbXlzcWwvTXlTcWxJbnZlbnRvcnlEdW1wZXJDcmVhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ine/core/metadata/model/PipelineTableMetaData.java](https://codecov.io/gh/apache/shardingsphere/pull/20777/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvbWV0YWRhdGEvbW9kZWwvUGlwZWxpbmVUYWJsZU1ldGFEYXRhLmphdmE=) | | |
| [...e/ingest/dumper/InventoryDumperCreatorFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/20777/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvaW5nZXN0L2R1bXBlci9JbnZlbnRvcnlEdW1wZXJDcmVhdG9yRmFjdG9yeS5qYXZh) | | |
| [...e/metadata/loader/PipelineTableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/20777/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvbWV0YWRhdGEvbG9hZGVyL1BpcGVsaW5lVGFibGVNZXRhRGF0YUxvYWRlci5qYXZh) | | |
| [.../pipeline/api/metadata/PipelineColumnMetaData.java](https://codecov.io/gh/apache/shardingsphere/pull/20777/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvYXBpL21ldGFkYXRhL1BpcGVsaW5lQ29sdW1uTWV0YURhdGEuamF2YQ==) | | |
| [...ingest/dumper/IncrementalDumperCreatorFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/20777/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvaW5nZXN0L2R1bXBlci9JbmNyZW1lbnRhbER1bXBlckNyZWF0b3JGYWN0b3J5LmphdmE=) | | |
| ... and [12 more](https://codecov.io/gh/apache/shardingsphere/pull/20777/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
: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] sunkai-cai commented on a diff in pull request #20777: Enhanced algorithm checking for CREATE SHARDING TABLE RULE
Posted by GitBox <gi...@apache.org>.
sunkai-cai commented on code in PR #20777:
URL: https://github.com/apache/shardingsphere/pull/20777#discussion_r963015708
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -84,8 +84,8 @@ public final class ShardingTableRuleStatementChecker {
/**
* Check create sharing table rule statement.
*
- * @param database database
Review Comment:
done it.
##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java:
##########
@@ -278,24 +278,35 @@ private static void checkTableRule(final String databaseName, final ShardingRule
}
private static void checkStrategy(final String databaseName, final ShardingRuleConfiguration currentRuleConfig, final Collection<TableRuleSegment> rules) throws DistSQLException {
- Collection<String> currentAlgorithms = null == currentRuleConfig ? Collections.emptySet() : currentRuleConfig.getShardingAlgorithms().keySet();
+ Map<String, AlgorithmConfiguration> currentAlgorithms = null == currentRuleConfig ? new HashMap<>() : currentRuleConfig.getShardingAlgorithms();
Collection<String> invalidAlgorithms = rules.stream().map(each -> Arrays.asList(each.getDatabaseStrategySegment(), each.getTableStrategySegment()))
.flatMap(Collection::stream).filter(Objects::nonNull).filter(each -> isInvalidStrategy(currentAlgorithms, each))
- .map(ShardingStrategySegment::getShardingAlgorithmName).collect(Collectors.toList());
+ .map(each -> null != each.getShardingAlgorithmName() ? each.getShardingAlgorithmName() : (null != each.getAlgorithmSegment() ? each.getAlgorithmSegment().getName() : ""))
+ .collect(Collectors.toList());
DistSQLException.predictionThrow(invalidAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException(databaseName, invalidAlgorithms));
}
- private static boolean isInvalidStrategy(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isInvalidStrategy(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
return !ShardingStrategyType.contain(shardingStrategySegment.getType())
|| !ShardingStrategyType.getValueOf(shardingStrategySegment.getType()).isValid(shardingStrategySegment.getShardingColumn())
- || !isAlgorithmExists(currentAlgorithms, shardingStrategySegment);
+ || !isStandardAlgorithmExists(currentAlgorithms, shardingStrategySegment);
}
- private static boolean isAlgorithmExists(final Collection<String> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ private static boolean isStandardAlgorithmExists(final Map<String, AlgorithmConfiguration> currentAlgorithms, final ShardingStrategySegment shardingStrategySegment) {
+ ShardingAlgorithm shardingAlgorithm;
if (null == shardingStrategySegment.getShardingAlgorithmName() && null != shardingStrategySegment.getAlgorithmSegment()) {
- return true;
+ shardingAlgorithm = ShardingAlgorithmFactory.newInstance(new AlgorithmConfiguration(
+ shardingStrategySegment.getAlgorithmSegment().getName(),
+ shardingStrategySegment.getAlgorithmSegment().getProps()));
+ } else {
+ AlgorithmConfiguration algorithmConfiguration = currentAlgorithms.get(shardingStrategySegment.getShardingAlgorithmName());
+ if (null == algorithmConfiguration) {
+ return false;
+ }
+ shardingAlgorithm = ShardingAlgorithmFactory.newInstance(algorithmConfiguration);
}
- return currentAlgorithms.contains(shardingStrategySegment.getShardingAlgorithmName());
+
Review Comment:
done 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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org