[shardingsphere] branch master updated: Optimize the verification of auto table algorithm type. (#22900)

[zh...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zhaojinchao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git


The following commit(s) were added to refs/heads/master by this push:
     new 5bd22fc3e8e Optimize the verification of auto table algorithm type. (#22900)
5bd22fc3e8e is described below

commit 5bd22fc3e8e19dc2b3abe77c4c731dd5dcc95e2e
Author: Raigor <ra...@gmail.com>
AuthorDate: Fri Dec 16 09:53:28 2022 +0800

    Optimize the verification of auto table algorithm type. (#22900)
---
 .../InvalidAlgorithmConfigurationException.java    |  4 ++++
 .../checker/ShardingTableRuleStatementChecker.java | 26 +++++++++++-----------
 .../checker/ShardingRuleStatementCheckerTest.java  | 16 +++++++++++++
 .../parser/segment/table/AutoTableRuleSegment.java |  9 --------
 4 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/distsql/handler/src/main/java/org/apache/shardingsphere/distsql/handler/exception/algorithm/InvalidAlgorithmConfigurationException.java b/distsql/handler/src/main/java/org/apache/shardingsphere/distsql/handler/exception/algorithm/InvalidAlgorithmConfigurationException.java
index d2355f75936..89a8a532402 100644
--- a/distsql/handler/src/main/java/org/apache/shardingsphere/distsql/handler/exception/algorithm/InvalidAlgorithmConfigurationException.java
+++ b/distsql/handler/src/main/java/org/apache/shardingsphere/distsql/handler/exception/algorithm/InvalidAlgorithmConfigurationException.java
@@ -33,6 +33,10 @@ public final class InvalidAlgorithmConfigurationException extends RuleDefinition
         super(XOpenSQLState.CHECK_OPTION_VIOLATION, 150, String.format("Invalid %s algorithms `%s`.", algorithmType, algorithms));
     }
     
+    public InvalidAlgorithmConfigurationException(final String algorithmType, final String algorithm, final String message) {
+        super(XOpenSQLState.CHECK_OPTION_VIOLATION, 150, String.format("Invalid %s algorithm `%s`, %s.", algorithmType, algorithm, message));
+    }
+    
     public InvalidAlgorithmConfigurationException(final String algorithmType, final String algorithm) {
         super(XOpenSQLState.CHECK_OPTION_VIOLATION, 150, String.format("Invalid %s algorithm `%s`.", algorithmType, algorithm));
     }
diff --git a/features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java b/features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java
index 827c16ffb92..e70c209c375 100644
--- a/features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java
+++ b/features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/checker/ShardingTableRuleStatementChecker.java
@@ -19,20 +19,19 @@ package org.apache.shardingsphere.sharding.distsql.handler.checker;
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
-import org.apache.shardingsphere.distsql.parser.segment.AlgorithmSegment;
-import org.apache.shardingsphere.infra.config.algorithm.AlgorithmConfiguration;
-import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.distsql.handler.exception.algorithm.InvalidAlgorithmConfigurationException;
+import org.apache.shardingsphere.distsql.handler.exception.algorithm.MissingRequiredAlgorithmException;
 import org.apache.shardingsphere.distsql.handler.exception.resource.MissingRequiredResourcesException;
 import org.apache.shardingsphere.distsql.handler.exception.rule.DuplicateRuleException;
-import org.apache.shardingsphere.distsql.handler.exception.algorithm.InvalidAlgorithmConfigurationException;
 import org.apache.shardingsphere.distsql.handler.exception.rule.InvalidRuleConfigurationException;
-import org.apache.shardingsphere.distsql.handler.exception.algorithm.MissingRequiredAlgorithmException;
 import org.apache.shardingsphere.distsql.handler.exception.rule.MissingRequiredRuleException;
+import org.apache.shardingsphere.distsql.parser.segment.AlgorithmSegment;
+import org.apache.shardingsphere.infra.config.algorithm.AlgorithmConfiguration;
+import org.apache.shardingsphere.infra.datanode.DataNode;
 import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
 import org.apache.shardingsphere.infra.rule.identifier.type.DataSourceContainedRule;
 import org.apache.shardingsphere.infra.util.exception.ShardingSpherePreconditions;
 import org.apache.shardingsphere.infra.util.expr.InlineExpressionParser;
-import org.apache.shardingsphere.infra.util.spi.type.typed.TypedSPI;
 import org.apache.shardingsphere.sharding.api.config.ShardingRuleConfiguration;
 import org.apache.shardingsphere.sharding.api.config.rule.ShardingAutoTableRuleConfiguration;
 import org.apache.shardingsphere.sharding.api.config.rule.ShardingTableReferenceRuleConfiguration;
@@ -277,13 +276,14 @@ public final class ShardingTableRuleStatementChecker {
     }
     
     private static void checkAutoTableShardingAlgorithms(final Collection<AutoTableRuleSegment> rules) {
-        ShardingSpherePreconditions.checkState(rules.stream().allMatch(AutoTableRuleSegment::isShardingAlgorithmCompleted), () -> new InvalidAlgorithmConfigurationException("sharding"));
-        Collection<String> invalidShardingAlgorithms = rules.stream().map(each -> each.getShardingAlgorithmSegment().getName()).distinct()
-                .filter(each -> !ShardingAlgorithmFactory.contains(each)).collect(Collectors.toList());
-        ShardingSpherePreconditions.checkState(invalidShardingAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException("sharding", invalidShardingAlgorithms));
-        invalidShardingAlgorithms.addAll(rules.stream().map(AutoTableRuleSegment::getShardingAlgorithmSegment).map(each -> new AlgorithmConfiguration(each.getName(), each.getProps()))
-                .map(ShardingAlgorithmFactory::newInstance).filter(each -> !(each instanceof ShardingAutoTableAlgorithm)).map(TypedSPI::getType).collect(Collectors.toList()));
-        ShardingSpherePreconditions.checkState(invalidShardingAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException("sharding", invalidShardingAlgorithms));
+        rules.forEach(each -> {
+            ShardingSpherePreconditions.checkState(ShardingAlgorithmFactory.contains(each.getShardingAlgorithmSegment().getName()),
+                    () -> new InvalidAlgorithmConfigurationException("sharding", each.getShardingAlgorithmSegment().getName()));
+            ShardingAlgorithm shardingAlgorithm = ShardingAlgorithmFactory.newInstance(
+                    new AlgorithmConfiguration(each.getShardingAlgorithmSegment().getName(), each.getShardingAlgorithmSegment().getProps()));
+            ShardingSpherePreconditions.checkState(shardingAlgorithm instanceof ShardingAutoTableAlgorithm, () -> new InvalidAlgorithmConfigurationException("sharding", shardingAlgorithm.getType(),
+                    String.format("auto sharding algorithm is required for rule `%s`", each.getLogicTable())));
+        });
     }
     
     private static void checkTableRule(final String databaseName, final Collection<AbstractTableRuleSegment> rules) {
diff --git a/features/sharding/distsql/handler/src/test/java/org/apache/shardingsphere/sharding/distsql/checker/ShardingRuleStatementCheckerTest.java b/features/sharding/distsql/handler/src/test/java/org/apache/shardingsphere/sharding/distsql/checker/ShardingRuleStatementCheckerTest.java
index 7682356c215..83b844f4cde 100644
--- a/features/sharding/distsql/handler/src/test/java/org/apache/shardingsphere/sharding/distsql/checker/ShardingRuleStatementCheckerTest.java
+++ b/features/sharding/distsql/handler/src/test/java/org/apache/shardingsphere/sharding/distsql/checker/ShardingRuleStatementCheckerTest.java
@@ -222,6 +222,22 @@ public final class ShardingRuleStatementCheckerTest {
         ShardingTableRuleStatementChecker.checkCreation(database, rules, shardingRuleConfig);
     }
     
+    @Test(expected = InvalidAlgorithmConfigurationException.class)
+    public void assertCheckAutoTableRuleWithStandardShardingAlgoithm() {
+        AutoTableRuleSegment autoTableRuleSegment = new AutoTableRuleSegment("t_product", Arrays.asList("ds_0", "ds_1"));
+        autoTableRuleSegment.setShardingAlgorithmSegment(new AlgorithmSegment("INLINE", newProperties("algorithm-expression", "ds_${product_id% 2}")));
+        List<AbstractTableRuleSegment> rules = Collections.singletonList(autoTableRuleSegment);
+        ShardingTableRuleStatementChecker.checkCreation(database, rules, shardingRuleConfig);
+    }
+    
+    @Test
+    public void assertCheckAutoTableRuleWithAutoShardingAlgoithm() {
+        AutoTableRuleSegment autoTableRuleSegment = new AutoTableRuleSegment("t_product", Arrays.asList("ds_0", "ds_1"));
+        autoTableRuleSegment.setShardingAlgorithmSegment(new AlgorithmSegment("MOD", newProperties("sharding-count", "4")));
+        List<AbstractTableRuleSegment> rules = Collections.singletonList(autoTableRuleSegment);
+        ShardingTableRuleStatementChecker.checkCreation(database, rules, shardingRuleConfig);
+    }
+    
     private static ShardingRuleConfiguration createShardingRuleConfiguration() {
         ShardingRuleConfiguration result = new ShardingRuleConfiguration();
         ShardingTableRuleConfiguration tableRuleConfig = new ShardingTableRuleConfiguration("t_order", "ds_${0..1}.t_order${0..1}");
diff --git a/features/sharding/distsql/statement/src/main/java/org/apache/shardingsphere/sharding/distsql/parser/segment/table/AutoTableRuleSegment.java b/features/sharding/distsql/statement/src/main/java/org/apache/shardingsphere/sharding/distsql/parser/segment/table/AutoTableRuleSegment.java
index 399cf3ea180..64f876f9e67 100644
--- a/features/sharding/distsql/statement/src/main/java/org/apache/shardingsphere/sharding/distsql/parser/segment/table/AutoTableRuleSegment.java
+++ b/features/sharding/distsql/statement/src/main/java/org/apache/shardingsphere/sharding/distsql/parser/segment/table/AutoTableRuleSegment.java
@@ -47,13 +47,4 @@ public final class AutoTableRuleSegment extends AbstractTableRuleSegment {
         this.shardingColumn = shardingColumn;
         this.shardingAlgorithmSegment = shardingAlgorithm;
     }
-    
-    /**
-     * Determine whether sharding algorithm completed.
-     *
-     * @return completed sharding algorithm or not
-     */
-    public boolean isShardingAlgorithmCompleted() {
-        return null != shardingColumn && null != shardingAlgorithmSegment;
-    }
 }