strongduanmu commented on a change in pull request #6997:
URL: https://github.com/apache/shardingsphere/pull/6997#discussion_r475168749
##########
File path:
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-route/src/main/java/org/apache/shardingsphere/sharding/route/engine/ShardingRouteDecorator.java
##########
@@ -70,9 +73,18 @@ public RouteContext decorate(final RouteContext
routeContext, final ShardingSphe
}
ShardingRouteEngine shardingRouteEngine =
ShardingRouteEngineFactory.newInstance(shardingRule, metaData,
sqlStatementContext, shardingConditions, props);
RouteResult routeResult = shardingRouteEngine.route(shardingRule);
+ if (containsUpdateDeletePagination(sqlStatement) &&
routeResult.getRouteUnits().size() > 1) {
Review comment:
> IMO, this condition judgment for `update and delete statements` seems
a little bit intrusive for `sharding router`. I mean it is a `detailed
statement judgment` for `router` seemingly. Do you think it is possible to move
this to the `statement validator`.
>
> However, the current `statement validator` maybe not meet our needs now.
Hence other changes for it are required. My feeling is that we can rename the
original `validate` function of `validator` as `preValidate` (A draft name,
welcome a better one) firstly. Moreover, this added validating condition could
be viewed as a `postValidate` function for `validator`.
>
> I'd like to listen to your voice.
> Bets,
> Trista
@tristaZero I really like the design of `preValidate` and `postValidate`,
which will make the logic clearer.
----------------------------------------------------------------
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:
[email protected]
