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/02/17 01:22:10 UTC
[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #14864: validate whether the sharding rules of the binding tables are consistent
strongduanmu commented on a change in pull request #14864:
URL: https://github.com/apache/shardingsphere/pull/14864#discussion_r808585189
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/rule/ShardingRule.java
##########
@@ -204,6 +208,105 @@ private BindingTableRule createBindingTableRule(final String bindingTableGroup)
return result;
}
+ private void checkSameBindingTables(final Collection<String> bindingTableGroups, final Map<String, BindingTableRule> bindingTableRules) {
+ if (null == bindingTableGroups || bindingTableGroups.isEmpty()) {
+ return;
+ }
+ for (String bindingTableGroup : bindingTableGroups) {
Review comment:
Please rename bindingTableGroup with each.
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/rule/ShardingRule.java
##########
@@ -204,6 +208,105 @@ private BindingTableRule createBindingTableRule(final String bindingTableGroup)
return result;
}
+ private void checkSameBindingTables(final Collection<String> bindingTableGroups, final Map<String, BindingTableRule> bindingTableRules) {
+ if (null == bindingTableGroups || bindingTableGroups.isEmpty()) {
+ return;
+ }
+ for (String bindingTableGroup : bindingTableGroups) {
+ List<String> bindingTableList = Splitter.on(",").trimResults().splitToList(bindingTableGroup.toLowerCase());
+ TableRule savedTableRule = null;
+ for (String bindingTable : bindingTableList) {
+ TableRule tableRule = bindingTableRules.get(bindingTable).getTableRules().get(bindingTable);
+ if (null == savedTableRule) {
+ savedTableRule = tableRule;
Review comment:
Do you think sampleTableRule is better?
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/rule/ShardingRule.java
##########
@@ -204,6 +208,105 @@ private BindingTableRule createBindingTableRule(final String bindingTableGroup)
return result;
}
+ private void checkSameBindingTables(final Collection<String> bindingTableGroups, final Map<String, BindingTableRule> bindingTableRules) {
+ if (null == bindingTableGroups || bindingTableGroups.isEmpty()) {
+ return;
+ }
+ for (String bindingTableGroup : bindingTableGroups) {
+ List<String> bindingTableList = Splitter.on(",").trimResults().splitToList(bindingTableGroup.toLowerCase());
+ TableRule savedTableRule = null;
+ for (String bindingTable : bindingTableList) {
+ TableRule tableRule = bindingTableRules.get(bindingTable).getTableRules().get(bindingTable);
+ if (null == savedTableRule) {
+ savedTableRule = tableRule;
+ } else {
+ checkSameActualDatasourceNamesAndActualTableIndex(savedTableRule, tableRule, bindingTableGroup);
+ }
+ }
+ }
+ }
+
+ private void checkSameActualDatasourceNamesAndActualTableIndex(final TableRule savedOne, final TableRule newOne, final String bindingTableGroup) {
Review comment:
Please rename savedOne to sampleTableRule, rename newOne to tableRule.
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/rule/ShardingRule.java
##########
@@ -204,6 +208,105 @@ private BindingTableRule createBindingTableRule(final String bindingTableGroup)
return result;
}
+ private void checkSameBindingTables(final Collection<String> bindingTableGroups, final Map<String, BindingTableRule> bindingTableRules) {
+ if (null == bindingTableGroups || bindingTableGroups.isEmpty()) {
Review comment:
@cheese8 Please remove this useless judge logic.
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/rule/ShardingRule.java
##########
@@ -204,6 +208,105 @@ private BindingTableRule createBindingTableRule(final String bindingTableGroup)
return result;
}
+ private void checkSameBindingTables(final Collection<String> bindingTableGroups, final Map<String, BindingTableRule> bindingTableRules) {
+ if (null == bindingTableGroups || bindingTableGroups.isEmpty()) {
+ return;
+ }
+ for (String bindingTableGroup : bindingTableGroups) {
+ List<String> bindingTableList = Splitter.on(",").trimResults().splitToList(bindingTableGroup.toLowerCase());
+ TableRule savedTableRule = null;
+ for (String bindingTable : bindingTableList) {
+ TableRule tableRule = bindingTableRules.get(bindingTable).getTableRules().get(bindingTable);
+ if (null == savedTableRule) {
+ savedTableRule = tableRule;
+ } else {
+ checkSameActualDatasourceNamesAndActualTableIndex(savedTableRule, tableRule, bindingTableGroup);
+ }
+ }
+ }
+ }
+
+ private void checkSameActualDatasourceNamesAndActualTableIndex(final TableRule savedOne, final TableRule newOne, final String bindingTableGroup) {
+ if (!savedOne.getActualDatasourceNames().containsAll(newOne.getActualDatasourceNames())) {
+ throw new ShardingSphereConfigurationException("The actualDatasourceNames on bindingTableGroup `%s` are inconsistent", bindingTableGroup);
+ }
+ checkSameAlgorithmOnDatabase(savedOne, newOne, savedOne.getActualDatasourceNames().stream().findFirst().get(), bindingTableGroup);
+ for (String dataSourceName : savedOne.getActualDatasourceNames()) {
+ Collection<String> savedActualTableNames = savedOne.getActualTableNames(dataSourceName).stream().map(each -> substring(each)[1])
+ .filter(each -> !each.isEmpty()).collect(Collectors.toList());
+ Collection<String> newOneActualTableNames = newOne.getActualTableNames(dataSourceName).stream().map(each -> substring(each)[1])
+ .filter(each -> !each.isEmpty()).collect(Collectors.toList());
+ if (!savedActualTableNames.containsAll(newOneActualTableNames)) {
+ throw new ShardingSphereConfigurationException("The actualTableNames on bindingTableGroup `%s` are inconsistent", bindingTableGroup);
+ }
+ checkSameAlgorithmOnTable(savedOne, savedOne.getActualTableNames(dataSourceName).stream().findFirst().get(), newOne,
+ newOne.getActualTableNames(dataSourceName).stream().findFirst().get(), bindingTableGroup);
+ }
+ }
+
+ private void checkSameAlgorithmOnDatabase(final TableRule savedOne, final TableRule newOne, final String dataSourceName,
+ final String bindingTableGroup) {
+ Collection<String[]> args = new ArrayList<>();
Review comment:
@cheese8 Please use more meaningful name for savedOne, newOne and args.
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/rule/ShardingRule.java
##########
@@ -204,6 +208,105 @@ private BindingTableRule createBindingTableRule(final String bindingTableGroup)
return result;
}
+ private void checkSameBindingTables(final Collection<String> bindingTableGroups, final Map<String, BindingTableRule> bindingTableRules) {
+ if (null == bindingTableGroups || bindingTableGroups.isEmpty()) {
+ return;
+ }
+ for (String bindingTableGroup : bindingTableGroups) {
+ List<String> bindingTableList = Splitter.on(",").trimResults().splitToList(bindingTableGroup.toLowerCase());
+ TableRule savedTableRule = null;
+ for (String bindingTable : bindingTableList) {
+ TableRule tableRule = bindingTableRules.get(bindingTable).getTableRules().get(bindingTable);
+ if (null == savedTableRule) {
+ savedTableRule = tableRule;
+ } else {
+ checkSameActualDatasourceNamesAndActualTableIndex(savedTableRule, tableRule, bindingTableGroup);
+ }
+ }
+ }
+ }
+
+ private void checkSameActualDatasourceNamesAndActualTableIndex(final TableRule savedOne, final TableRule newOne, final String bindingTableGroup) {
+ if (!savedOne.getActualDatasourceNames().containsAll(newOne.getActualDatasourceNames())) {
+ throw new ShardingSphereConfigurationException("The actualDatasourceNames on bindingTableGroup `%s` are inconsistent", bindingTableGroup);
Review comment:
Please split word or exception message.
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/rule/ShardingRule.java
##########
@@ -204,6 +208,105 @@ private BindingTableRule createBindingTableRule(final String bindingTableGroup)
return result;
}
+ private void checkSameBindingTables(final Collection<String> bindingTableGroups, final Map<String, BindingTableRule> bindingTableRules) {
+ if (null == bindingTableGroups || bindingTableGroups.isEmpty()) {
+ return;
+ }
+ for (String bindingTableGroup : bindingTableGroups) {
+ List<String> bindingTableList = Splitter.on(",").trimResults().splitToList(bindingTableGroup.toLowerCase());
+ TableRule savedTableRule = null;
+ for (String bindingTable : bindingTableList) {
+ TableRule tableRule = bindingTableRules.get(bindingTable).getTableRules().get(bindingTable);
+ if (null == savedTableRule) {
+ savedTableRule = tableRule;
+ } else {
+ checkSameActualDatasourceNamesAndActualTableIndex(savedTableRule, tableRule, bindingTableGroup);
+ }
+ }
+ }
+ }
+
+ private void checkSameActualDatasourceNamesAndActualTableIndex(final TableRule savedOne, final TableRule newOne, final String bindingTableGroup) {
+ if (!savedOne.getActualDatasourceNames().containsAll(newOne.getActualDatasourceNames())) {
+ throw new ShardingSphereConfigurationException("The actualDatasourceNames on bindingTableGroup `%s` are inconsistent", bindingTableGroup);
+ }
+ checkSameAlgorithmOnDatabase(savedOne, newOne, savedOne.getActualDatasourceNames().stream().findFirst().get(), bindingTableGroup);
+ for (String dataSourceName : savedOne.getActualDatasourceNames()) {
+ Collection<String> savedActualTableNames = savedOne.getActualTableNames(dataSourceName).stream().map(each -> substring(each)[1])
+ .filter(each -> !each.isEmpty()).collect(Collectors.toList());
+ Collection<String> newOneActualTableNames = newOne.getActualTableNames(dataSourceName).stream().map(each -> substring(each)[1])
+ .filter(each -> !each.isEmpty()).collect(Collectors.toList());
+ if (!savedActualTableNames.containsAll(newOneActualTableNames)) {
Review comment:
@cheese8 Is this logic correct? The real table corresponding to the binding table should be different.
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/rule/ShardingRule.java
##########
@@ -204,6 +208,105 @@ private BindingTableRule createBindingTableRule(final String bindingTableGroup)
return result;
}
+ private void checkSameBindingTables(final Collection<String> bindingTableGroups, final Map<String, BindingTableRule> bindingTableRules) {
+ if (null == bindingTableGroups || bindingTableGroups.isEmpty()) {
+ return;
+ }
+ for (String bindingTableGroup : bindingTableGroups) {
+ List<String> bindingTableList = Splitter.on(",").trimResults().splitToList(bindingTableGroup.toLowerCase());
+ TableRule savedTableRule = null;
+ for (String bindingTable : bindingTableList) {
+ TableRule tableRule = bindingTableRules.get(bindingTable).getTableRules().get(bindingTable);
+ if (null == savedTableRule) {
+ savedTableRule = tableRule;
+ } else {
+ checkSameActualDatasourceNamesAndActualTableIndex(savedTableRule, tableRule, bindingTableGroup);
+ }
+ }
+ }
+ }
+
+ private void checkSameActualDatasourceNamesAndActualTableIndex(final TableRule savedOne, final TableRule newOne, final String bindingTableGroup) {
+ if (!savedOne.getActualDatasourceNames().containsAll(newOne.getActualDatasourceNames())) {
+ throw new ShardingSphereConfigurationException("The actualDatasourceNames on bindingTableGroup `%s` are inconsistent", bindingTableGroup);
+ }
+ checkSameAlgorithmOnDatabase(savedOne, newOne, savedOne.getActualDatasourceNames().stream().findFirst().get(), bindingTableGroup);
+ for (String dataSourceName : savedOne.getActualDatasourceNames()) {
Review comment:
Please rename dataSourceName to 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