strongduanmu commented on a change in pull request #11750:
URL: https://github.com/apache/shardingsphere/pull/11750#discussion_r686544398



##########
File path: 
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/route/engine/validator/dml/impl/ShardingInsertStatementValidator.java
##########
@@ -92,10 +92,20 @@ private boolean isAllSameTables(final Collection<String> 
tableNames) {
     }
     
     @Override
-    public void postValidate(final ShardingRule shardingRule, final 
SQLStatementContext<InsertStatement> sqlStatementContext, 
+    public void postValidate(final ShardingRule shardingRule, final 
SQLStatementContext<InsertStatement> sqlStatementContext,
                              final RouteContext routeContext, final 
ShardingSphereSchema schema) {
         if (needCheckDatabaseInstance) {
             Preconditions.checkState(routeContext.isSingleRouting(), "Sharding 
value must same with subquery.");
         }
+        if (routeContext.isSingleRouting()) {
+            return;
+        }
+        String tableName = 
sqlStatementContext.getSqlStatement().getTable().getTableName().getIdentifier().getValue();
+        if (shardingRule.isBroadcastTable(tableName)) {
+            return;
+        }
+        if (routeContext.getOriginalDataNodes().stream().anyMatch(dataNodes -> 
dataNodes.size() > 1)) {
+            throw new ShardingSphereException("Insert clause not support 
routing to multiple dataNodes when is not broadcastTable.");

Review comment:
       @chengh1 `Insert statement does not support sharding table routing to 
multiple data nodes.`. Do you think this exception message is better?

##########
File path: 
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/validator/dml/ShardingInsertStatementValidatorTest.java
##########
@@ -130,6 +135,62 @@ public void assertValidateInsertSelectWithBindingTables() {
         new ShardingInsertStatementValidator().preValidate(shardingRule, 
sqlStatementContext, Collections.emptyList(), mock(ShardingSphereSchema.class));
     }
     
+    @Test
+    public void assertValidateInsertWithSingleRouting() {
+        SQLStatementContext<InsertStatement> sqlStatementContext = 
createInsertStatementContext(Collections.singletonList(1), 
createInsertStatement());
+        when(routeContext.isSingleRouting()).thenReturn(true);
+        new ShardingInsertStatementValidator().postValidate(shardingRule, 
sqlStatementContext, routeContext, mock(ShardingSphereSchema.class));
+    }
+    
+    @Test
+    public void assertValidateInsertWithBroadcastTable() {
+        SQLStatementContext<InsertStatement> sqlStatementContext = 
createInsertStatementContext(Collections.singletonList(1), 
createInsertStatement());
+        when(routeContext.isSingleRouting()).thenReturn(false);
+        
when(shardingRule.isBroadcastTable(sqlStatementContext.getSqlStatement().getTable().getTableName().getIdentifier().getValue())).thenReturn(true);
+        new ShardingInsertStatementValidator().postValidate(shardingRule, 
sqlStatementContext, routeContext, mock(ShardingSphereSchema.class));
+    }
+    
+    @Test
+    public void assertValidateInsertWithRoutingToSingleDataNode() {
+        SQLStatementContext<InsertStatement> sqlStatementContext = 
createInsertStatementContext(Collections.singletonList(1), 
createInsertStatement());
+        when(routeContext.isSingleRouting()).thenReturn(false);
+        
when(shardingRule.isBroadcastTable(sqlStatementContext.getSqlStatement().getTable().getTableName().getIdentifier().getValue())).thenReturn(false);
+        
when(routeContext.getOriginalDataNodes()).thenReturn(getSingleRouteDataNodes());
+        new ShardingInsertStatementValidator().postValidate(shardingRule, 
sqlStatementContext, routeContext, mock(ShardingSphereSchema.class));
+    }
+    
+    @Test(expected = ShardingSphereException.class)
+    public void assertValidateInsertWithRoutingToMultipleDataNodes() {
+        SQLStatementContext<InsertStatement> sqlStatementContext = 
createInsertStatementContext(Collections.singletonList(1), 
createInsertStatement());
+        when(routeContext.isSingleRouting()).thenReturn(false);
+        
when(shardingRule.isBroadcastTable(sqlStatementContext.getSqlStatement().getTable().getTableName().getIdentifier().getValue())).thenReturn(false);
+        
when(routeContext.getOriginalDataNodes()).thenReturn(getMultipleRouteDataNodes());
+        new ShardingInsertStatementValidator().postValidate(shardingRule, 
sqlStatementContext, routeContext, mock(ShardingSphereSchema.class));
+    }
+    
+    private Collection<Collection<DataNode>> getMultipleRouteDataNodes() {
+        Collection<DataNode> value1DataNodes = new LinkedList<>();
+        value1DataNodes.add(new DataNode("ds_0", "user_0"));
+        Collection<DataNode> value2DataNodes = new LinkedList<>();
+        value2DataNodes.add(new DataNode("ds_0", "user_0"));
+        value2DataNodes.add(new DataNode("ds_0", "user_1"));
+        Collection<Collection<DataNode>> originalDataNodes = new 
LinkedList<>();
+        originalDataNodes.add(value1DataNodes);
+        originalDataNodes.add(value2DataNodes);
+        return originalDataNodes;

Review comment:
       @chengh1 change it to `result` may be better.

##########
File path: 
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/validator/dml/ShardingInsertStatementValidatorTest.java
##########
@@ -130,6 +135,62 @@ public void assertValidateInsertSelectWithBindingTables() {
         new ShardingInsertStatementValidator().preValidate(shardingRule, 
sqlStatementContext, Collections.emptyList(), mock(ShardingSphereSchema.class));
     }
     
+    @Test
+    public void assertValidateInsertWithSingleRouting() {
+        SQLStatementContext<InsertStatement> sqlStatementContext = 
createInsertStatementContext(Collections.singletonList(1), 
createInsertStatement());
+        when(routeContext.isSingleRouting()).thenReturn(true);
+        new ShardingInsertStatementValidator().postValidate(shardingRule, 
sqlStatementContext, routeContext, mock(ShardingSphereSchema.class));
+    }
+    
+    @Test
+    public void assertValidateInsertWithBroadcastTable() {
+        SQLStatementContext<InsertStatement> sqlStatementContext = 
createInsertStatementContext(Collections.singletonList(1), 
createInsertStatement());
+        when(routeContext.isSingleRouting()).thenReturn(false);
+        
when(shardingRule.isBroadcastTable(sqlStatementContext.getSqlStatement().getTable().getTableName().getIdentifier().getValue())).thenReturn(true);
+        new ShardingInsertStatementValidator().postValidate(shardingRule, 
sqlStatementContext, routeContext, mock(ShardingSphereSchema.class));
+    }
+    
+    @Test
+    public void assertValidateInsertWithRoutingToSingleDataNode() {
+        SQLStatementContext<InsertStatement> sqlStatementContext = 
createInsertStatementContext(Collections.singletonList(1), 
createInsertStatement());
+        when(routeContext.isSingleRouting()).thenReturn(false);
+        
when(shardingRule.isBroadcastTable(sqlStatementContext.getSqlStatement().getTable().getTableName().getIdentifier().getValue())).thenReturn(false);
+        
when(routeContext.getOriginalDataNodes()).thenReturn(getSingleRouteDataNodes());
+        new ShardingInsertStatementValidator().postValidate(shardingRule, 
sqlStatementContext, routeContext, mock(ShardingSphereSchema.class));
+    }
+    
+    @Test(expected = ShardingSphereException.class)
+    public void assertValidateInsertWithRoutingToMultipleDataNodes() {
+        SQLStatementContext<InsertStatement> sqlStatementContext = 
createInsertStatementContext(Collections.singletonList(1), 
createInsertStatement());
+        when(routeContext.isSingleRouting()).thenReturn(false);
+        
when(shardingRule.isBroadcastTable(sqlStatementContext.getSqlStatement().getTable().getTableName().getIdentifier().getValue())).thenReturn(false);
+        
when(routeContext.getOriginalDataNodes()).thenReturn(getMultipleRouteDataNodes());
+        new ShardingInsertStatementValidator().postValidate(shardingRule, 
sqlStatementContext, routeContext, mock(ShardingSphereSchema.class));
+    }
+    
+    private Collection<Collection<DataNode>> getMultipleRouteDataNodes() {
+        Collection<DataNode> value1DataNodes = new LinkedList<>();
+        value1DataNodes.add(new DataNode("ds_0", "user_0"));
+        Collection<DataNode> value2DataNodes = new LinkedList<>();
+        value2DataNodes.add(new DataNode("ds_0", "user_0"));
+        value2DataNodes.add(new DataNode("ds_0", "user_1"));
+        Collection<Collection<DataNode>> originalDataNodes = new 
LinkedList<>();
+        originalDataNodes.add(value1DataNodes);
+        originalDataNodes.add(value2DataNodes);
+        return originalDataNodes;
+    }
+    
+    private Collection<Collection<DataNode>> getSingleRouteDataNodes() {
+        Collection<DataNode> value1DataNodes = new LinkedList<>();
+        value1DataNodes.add(new DataNode("ds_0", "user_0"));
+        Collection<DataNode> value2DataNodes = new LinkedList<>();
+        value2DataNodes.add(new DataNode("ds_0", "user_0"));
+        Collection<Collection<DataNode>> originalDataNodes = new 
LinkedList<>();
+        originalDataNodes.add(value1DataNodes);
+        originalDataNodes.add(value2DataNodes);
+        return originalDataNodes;

Review comment:
       Same problem.




-- 
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