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

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

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



##########
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:
       > 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?
   
   @tristaZero I agree with you that the inclusion of SQL in the exception is indeed a bit verbose, especially when the SQL is very complex.
   




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