tristaZero commented on a change in pull request #6997:
URL: https://github.com/apache/shardingsphere/pull/6997#discussion_r475161148
##########
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
##########
File path:
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-route/src/main/java/org/apache/shardingsphere/sharding/route/engine/validator/impl/ShardingInsertStatementValidator.java
##########
@@ -48,12 +48,12 @@ public void validate(final ShardingRule shardingRule, final
SQLStatementContext<
Optional<OnDuplicateKeyColumnsSegment> onDuplicateKeyColumnsSegment =
sqlStatement.getOnDuplicateKeyColumns();
String tableName =
sqlStatement.getTable().getTableName().getIdentifier().getValue();
if (onDuplicateKeyColumnsSegment.isPresent() &&
isUpdateShardingKey(shardingRule, onDuplicateKeyColumnsSegment.get(),
tableName)) {
- throw new ShardingSphereException("INSERT INTO .... ON DUPLICATE
KEY UPDATE can not support update for sharding column.");
+ throw new ShardingSphereException("INSERT INTO ... ON DUPLICATE
KEY UPDATE can not support update for sharding column.");
Review comment:
Polishing work, great. :-)
----------------------------------------------------------------
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]
