You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by ji...@apache.org on 2023/04/12 06:39:56 UTC

[shardingsphere] branch master updated: Remove redundant check in shadow rule (#25129)

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

jianglongtao 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 ab993713b0c Remove redundant check in shadow rule (#25129)
ab993713b0c is described below

commit ab993713b0c060475074ccb88501a8955f8e4d26
Author: ChenJiaHao <Pa...@163.com>
AuthorDate: Wed Apr 12 14:39:47 2023 +0800

    Remove redundant check in shadow rule (#25129)
    
    * Remove redundant check in shadow rule
    
    * Fix UT
---
 .../handler/checker/ShadowRuleStatementChecker.java       | 15 +--------------
 .../handler/update/AlterShadowRuleStatementUpdater.java   |  2 --
 .../handler/update/CreateShadowRuleStatementUpdater.java  |  1 -
 .../update/AlterShadowRuleStatementUpdaterTest.java       | 10 ++++++++++
 .../update/CreateShadowRuleStatementUpdaterTest.java      | 10 +++++++++-
 .../distsql/parser/segment/ShadowAlgorithmSegment.java    |  8 --------
 6 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/checker/ShadowRuleStatementChecker.java b/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/checker/ShadowRuleStatementChecker.java
index 150c0792920..d5ee95a261c 100644
--- a/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/checker/ShadowRuleStatementChecker.java
+++ b/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/checker/ShadowRuleStatementChecker.java
@@ -18,17 +18,14 @@
 package org.apache.shardingsphere.shadow.distsql.handler.checker;
 
 import org.apache.shardingsphere.distsql.handler.exception.DistSQLException;
-import org.apache.shardingsphere.distsql.handler.exception.algorithm.InvalidAlgorithmConfigurationException;
-import org.apache.shardingsphere.distsql.handler.exception.storageunit.MissingRequiredStorageUnitsException;
 import org.apache.shardingsphere.distsql.handler.exception.rule.MissingRequiredRuleException;
+import org.apache.shardingsphere.distsql.handler.exception.storageunit.MissingRequiredStorageUnitsException;
 import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
 import org.apache.shardingsphere.infra.util.exception.ShardingSpherePreconditions;
 import org.apache.shardingsphere.shadow.api.config.ShadowRuleConfiguration;
-import org.apache.shardingsphere.shadow.distsql.parser.segment.ShadowAlgorithmSegment;
 
 import java.util.Collection;
 import java.util.Map;
-import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -58,16 +55,6 @@ public class ShadowRuleStatementChecker {
         ShardingSpherePreconditions.checkState(notExistedStorageUnits.isEmpty(), () -> new MissingRequiredStorageUnitsException(database.getName(), notExistedStorageUnits));
     }
     
-    /**
-     * Check the completeness of the algorithms.
-     *
-     * @param algorithmSegments to be checked segments
-     */
-    public static void checkAlgorithmCompleteness(final Collection<ShadowAlgorithmSegment> algorithmSegments) {
-        Set<ShadowAlgorithmSegment> incompleteAlgorithms = algorithmSegments.stream().filter(each -> !each.isComplete()).collect(Collectors.toSet());
-        ShardingSpherePreconditions.checkState(incompleteAlgorithms.isEmpty(), () -> new InvalidAlgorithmConfigurationException("shadow"));
-    }
-    
     /**
      * Check if there are duplicated rules.
      * 
diff --git a/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/AlterShadowRuleStatementUpdater.java b/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/AlterShadowRuleStatementUpdater.java
index f0bcd69e5cf..752fb69544d 100644
--- a/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/AlterShadowRuleStatementUpdater.java
+++ b/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/AlterShadowRuleStatementUpdater.java
@@ -90,8 +90,6 @@ public final class AlterShadowRuleStatementUpdater implements RuleDefinitionAlte
     }
     
     private void checkAlgorithms(final String databaseName, final Collection<ShadowRuleSegment> segments) {
-        Collection<ShadowAlgorithmSegment> shadowAlgorithmSegments = ShadowRuleStatementSupporter.getShadowAlgorithmSegment(segments);
-        ShadowRuleStatementChecker.checkAlgorithmCompleteness(shadowAlgorithmSegments);
         Collection<String> requiredAlgorithms = ShadowRuleStatementSupporter.getAlgorithmNames(segments);
         ShadowRuleStatementChecker.checkDuplicated(requiredAlgorithms, duplicated -> new AlgorithmInUsedException("Shadow", databaseName, duplicated));
     }
diff --git a/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/CreateShadowRuleStatementUpdater.java b/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/CreateShadowRuleStatementUpdater.java
index 3de3e29b5ab..b07c1f145e7 100644
--- a/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/CreateShadowRuleStatementUpdater.java
+++ b/features/shadow/distsql/handler/src/main/java/org/apache/shardingsphere/shadow/distsql/handler/update/CreateShadowRuleStatementUpdater.java
@@ -86,7 +86,6 @@ public final class CreateShadowRuleStatementUpdater implements RuleDefinitionCre
     }
     
     private void checkAlgorithms(final String databaseName, final Collection<ShadowRuleSegment> segments) {
-        ShadowRuleStatementChecker.checkAlgorithmCompleteness(ShadowRuleStatementSupporter.getShadowAlgorithmSegment(segments));
         ShadowRuleStatementChecker.checkDuplicated(ShadowRuleStatementSupporter.getAlgorithmNames(segments), duplicated -> new DuplicateRuleException("shadow", databaseName, duplicated));
     }
     
diff --git a/features/shadow/distsql/handler/src/test/java/org/apache/shardingsphere/shadow/distsql/update/AlterShadowRuleStatementUpdaterTest.java b/features/shadow/distsql/handler/src/test/java/org/apache/shardingsphere/shadow/distsql/update/AlterShadowRuleStatementUpdaterTest.java
index 0e7b101841f..64973e4401d 100644
--- a/features/shadow/distsql/handler/src/test/java/org/apache/shardingsphere/shadow/distsql/update/AlterShadowRuleStatementUpdaterTest.java
+++ b/features/shadow/distsql/handler/src/test/java/org/apache/shardingsphere/shadow/distsql/update/AlterShadowRuleStatementUpdaterTest.java
@@ -130,4 +130,14 @@ class AlterShadowRuleStatementUpdaterTest {
                 new ShadowRuleSegment("initRuleName2", "ds1", null, Collections.singletonMap("t_order_1", Collections.singletonList(segment2)))));
         updater.checkSQLStatement(database, sqlStatement, currentConfig);
     }
+    
+    @Test
+    void assertExecuteSuccessWithoutProps() {
+        ShadowAlgorithmSegment segment1 = new ShadowAlgorithmSegment("algorithmName1", new AlgorithmSegment("SQL_HINT", null));
+        ShadowAlgorithmSegment segment2 = new ShadowAlgorithmSegment("algorithmName2", new AlgorithmSegment("SQL_HINT", null));
+        AlterShadowRuleStatement sqlStatement = new AlterShadowRuleStatement(Arrays.asList(
+                new ShadowRuleSegment("initRuleName1", "ds", null, Collections.singletonMap("t_order", Collections.singleton(segment1))),
+                new ShadowRuleSegment("initRuleName2", "ds1", null, Collections.singletonMap("t_order_1", Collections.singletonList(segment2)))));
+        updater.checkSQLStatement(database, sqlStatement, currentConfig);
+    }
 }
diff --git a/features/shadow/distsql/handler/src/test/java/org/apache/shardingsphere/shadow/distsql/update/CreateShadowRuleStatementUpdaterTest.java b/features/shadow/distsql/handler/src/test/java/org/apache/shardingsphere/shadow/distsql/update/CreateShadowRuleStatementUpdaterTest.java
index 0b47bb4d6db..3958d00868c 100644
--- a/features/shadow/distsql/handler/src/test/java/org/apache/shardingsphere/shadow/distsql/update/CreateShadowRuleStatementUpdaterTest.java
+++ b/features/shadow/distsql/handler/src/test/java/org/apache/shardingsphere/shadow/distsql/update/CreateShadowRuleStatementUpdaterTest.java
@@ -65,7 +65,7 @@ class CreateShadowRuleStatementUpdaterTest {
     @BeforeEach
     void before() {
         when(database.getResourceMetaData()).thenReturn(resourceMetaData);
-        when(database.getName()).thenReturn("aa");
+        when(database.getName()).thenReturn("shadow_db");
         when(currentConfig.getDataSources()).thenReturn(Collections.singleton(new ShadowDataSourceConfiguration("initRuleName", "initDs0", "initDs0Shadow")));
     }
     
@@ -115,6 +115,14 @@ class CreateShadowRuleStatementUpdaterTest {
         assertThrows(ServiceProviderNotFoundServerException.class, () -> updater.checkSQLStatement(database, sqlStatement, currentConfig));
     }
     
+    @Test
+    void assertExecuteWithoutProps() {
+        ShadowAlgorithmSegment segment = new ShadowAlgorithmSegment("algorithmName", new AlgorithmSegment("SQL_HINT", null));
+        CreateShadowRuleStatement sqlStatement = new CreateShadowRuleStatement(false,
+                Collections.singleton(new ShadowRuleSegment("initRuleNameWithoutProps", "ds", null, Collections.singletonMap("t_order", Collections.singleton(segment)))));
+        updater.checkSQLStatement(database, sqlStatement, currentConfig);
+    }
+    
     @Test
     void assertExecuteWithIfNotExists() {
         ShadowAlgorithmSegment segment = new ShadowAlgorithmSegment("algorithmName", new AlgorithmSegment("SQL_HINT", PropertiesBuilder.build(new Property("type", "value"))));
diff --git a/features/shadow/distsql/statement/src/main/java/org/apache/shardingsphere/shadow/distsql/parser/segment/ShadowAlgorithmSegment.java b/features/shadow/distsql/statement/src/main/java/org/apache/shardingsphere/shadow/distsql/parser/segment/ShadowAlgorithmSegment.java
index 2da065aaa17..5024c19db40 100644
--- a/features/shadow/distsql/statement/src/main/java/org/apache/shardingsphere/shadow/distsql/parser/segment/ShadowAlgorithmSegment.java
+++ b/features/shadow/distsql/statement/src/main/java/org/apache/shardingsphere/shadow/distsql/parser/segment/ShadowAlgorithmSegment.java
@@ -32,12 +32,4 @@ public final class ShadowAlgorithmSegment implements ASTNode {
     private final String algorithmName;
     
     private final AlgorithmSegment algorithmSegment;
-    
-    /**
-     * Check for completeness.
-     * @return complete or not
-     */
-    public boolean isComplete() {
-        return !getAlgorithmName().isEmpty() && !getAlgorithmSegment().getName().isEmpty() && !getAlgorithmSegment().getProps().isEmpty();
-    }
 }