strongduanmu commented on a change in pull request #6377:
URL: https://github.com/apache/shardingsphere/pull/6377#discussion_r456856372
##########
File path:
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-rewrite/src/main/java/org/apache/shardingsphere/sharding/rewrite/context/ShardingSQLRewriteContextDecorator.java
##########
@@ -38,8 +42,15 @@
@Override
public void decorate(final ShardingRule shardingRule, final
ConfigurationProperties props, final SQLRewriteContext sqlRewriteContext, final
RouteContext routeContext) {
for (ParameterRewriter each : new
ShardingParameterRewriterBuilder(shardingRule,
routeContext).getParameterRewriters(sqlRewriteContext.getSchemaMetaData())) {
- if (!sqlRewriteContext.getParameters().isEmpty() &&
each.isNeedRewrite(sqlRewriteContext.getSqlStatementContext())) {
- each.rewrite(sqlRewriteContext.getParameterBuilder(),
sqlRewriteContext.getSqlStatementContext(), sqlRewriteContext.getParameters());
+ SQLStatementContext sqlStatementContext =
sqlRewriteContext.getSqlStatementContext();
+ if (!sqlRewriteContext.getParameters().isEmpty() &&
each.isNeedRewrite(sqlStatementContext)) {
+ each.rewrite(sqlRewriteContext.getParameterBuilder(),
sqlStatementContext, sqlRewriteContext.getParameters());
+ }
+ if (sqlStatementContext instanceof InsertStatementContext && null
!= ((InsertStatementContext) sqlStatementContext).getInsertSelectContext()
+ && !sqlRewriteContext.getParameters().isEmpty() &&
each.isNeedRewrite(((InsertStatementContext)
sqlStatementContext).getInsertSelectContext().getSelectStatementContext())) {
+ StandardParameterBuilder parameterBuilder =
sqlRewriteContext.getParameterBuilder() instanceof GroupedParameterBuilder
Review comment:
> It is impossible to have many `values(), ()` in `INSERT SELECT`, isn't
it?
> If it is that case, there is no chance for `GroupedParameterBuilder` here,
right?
> Do you think no estimation for'GroupedParameterBuilder` is reasonable?
There are two reasons for this realization:
1. When initializing SQLRewriteContext, if it is InsertStatementContext,
GroupedParameterBuilder will be used to manage parameters.
If the StandardParameterBuilder is used for the insert select statement, it
will be necessary to determine whether it is an insert select statement during
the rewrite process, and then decide whether to take the value from the
StandardParameterBuilder or the GroupedParameterBuilder.
2. Since the select union syntax is not supported now, only one set of
select statement parameters will be stored in the GroupedParameterBuilder.
After the select union syntax is supported in the future, there will be
scenarios containing multiple sets of select statement parameters.
Maybe it's better to add a FIXME here. If we support select union in the
future, then modify the rewrite function to support GroupedParameterBuilder
parameter.
----------------------------------------------------------------
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]
