[GitHub] [shardingsphere] tristaZero commented on a change in pull request #7796: fix multi-table validation error for DML modify statements

[GitBox <gi...@apache.org>]

tristaZero commented on a change in pull request #7796:
URL: https://github.com/apache/shardingsphere/pull/7796#discussion_r505475984



##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-route/src/main/java/org/apache/shardingsphere/sharding/route/engine/validator/dml/ShardingDMLStatementValidator.java
##########
@@ -31,11 +34,14 @@
     /**
      * Validate multiple table.
      *
+     * @param shardingRule sharding rule
      * @param sqlStatementContext sqlStatementContext
+     * @param sql sql
      */
-    protected void validateMultipleTable(final SQLStatementContext<T> sqlStatementContext) {
-        if (1 != ((TableAvailable) sqlStatementContext).getAllTables().size()) {
-            throw new ShardingSphereException("Cannot support Multiple-Table for '%s'.", sqlStatementContext.getSqlStatement());
+    protected void validateMultipleTable(final ShardingRule shardingRule, final SQLStatementContext<T> sqlStatementContext, final String sql) {

Review comment:
       1. From my perspective, It is unnecessary to have SQL as an input parameter. Precisely speaking, It is a little heavy to pass SQL just for an exception. Maybe `Cannot support such a Multiple-Table query for '$tableNames'` is enough. What do you think?
   2. The constrictions for multi-tables looks like incomplete? How about broadcast tables or the single table in the same instance with sharding tables (Maybe a postValidate()?). 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org