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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to