strongduanmu commented on a change in pull request #11535:
URL: https://github.com/apache/shardingsphere/pull/11535#discussion_r678953162
##########
File path:
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/route/engine/validator/dml/impl/ShardingSelectStatementValidator.java
##########
@@ -40,6 +41,11 @@ public void preValidate(final ShardingRule shardingRule,
final SQLStatementConte
if (isNeedMergeShardingValues(sqlStatementContext, shardingRule)) {
needCheckDatabaseInstance =
checkSubqueryShardingValues(shardingRule, sqlStatementContext, parameters,
schema);
}
+ if
(!sqlStatementContext.getSqlStatement().getUnionSegments().isEmpty()) {
+ if
(!shardingRule.isAllBindingTables(sqlStatementContext.getTablesContext().getTableNames())
&&
!shardingRule.isAllSingleTables(sqlStatementContext.getTablesContext().getTableNames()))
{
+ throw new ShardingSphereException("UNION only support all
binding tables or all single tables.");
Review comment:
@tuichenchuxin `SELECT ... UNION statement can not support none binding
tables.` may be better.
##########
File path:
shardingsphere-test/shardingsphere-rewrite-test/src/test/resources/scenario/sharding/case/select.xml
##########
@@ -477,4 +476,19 @@
<input sql="SELECT * FROM t_account join t_account_detail on
t_account.account_id = t_account_detail.account_id and t_account.account_id in
( ? )" parameters="1"/>
<output sql="SELECT * FROM t_account_1 join t_account_detail_1 on
t_account_1.account_id = t_account_detail_1.account_id and
t_account_1.account_id in ( ? )" parameters="1"/>
</rewrite-assertion>
+
+ <rewrite-assertion id="select_union_table_one_data_node" db-type="MySQL">
+ <input sql="SELECT order_id FROM t_order WHERE order_id = 1 UNION
SELECT order_id FROM t_order_item WHERE order_id = 1"/>
+ <output sql="SELECT order_id FROM t_order_0 WHERE order_id = 1 UNION
SELECT order_id FROM t_order_item_0 WHERE order_id = 1"/>
+ </rewrite-assertion>
+
+ <rewrite-assertion id="select_union_bind_table" db-type="MySQL">
+ <input sql="SELECT account_id FROM t_account WHERE account_id = 1
UNION SELECT account_id FROM t_account_detail"/>
+ <output sql="SELECT account_id FROM t_account_1 WHERE account_id = 1
UNION SELECT account_id FROM t_account_detail_1"/>
Review comment:
@tuichenchuxin This case is wrong, please modify check logic to avoid
sharding tables.
##########
File path:
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/route/engine/validator/dml/impl/ShardingSelectStatementValidator.java
##########
@@ -40,6 +41,11 @@ public void preValidate(final ShardingRule shardingRule,
final SQLStatementConte
if (isNeedMergeShardingValues(sqlStatementContext, shardingRule)) {
needCheckDatabaseInstance =
checkSubqueryShardingValues(shardingRule, sqlStatementContext, parameters,
schema);
}
+ if
(!sqlStatementContext.getSqlStatement().getUnionSegments().isEmpty()) {
Review comment:
@tuichenchuxin Please extract this logic to a method, and remove
`isAllSingleTables` method.
##########
File path:
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/route/engine/validator/dml/impl/ShardingSelectStatementValidator.java
##########
@@ -48,5 +54,8 @@ public void postValidate(final ShardingRule shardingRule,
final SQLStatementCont
if (!routeContext.isFederated() && needCheckDatabaseInstance) {
Preconditions.checkState(routeContext.isSingleRouting(), "Sharding
value must same with subquery.");
}
+ if
(!sqlStatementContext.getSqlStatement().getUnionSegments().isEmpty() &&
routeContext.getRouteUnits().size() > 1) {
+ throw new ShardingSphereException("UNION can not support sharding
route to multiple data nodes.");
Review comment:
@tuichenchuxin `SELECT ... UNION can not support sharding route to
multiple data nodes.`
##########
File path:
shardingsphere-test/shardingsphere-rewrite-test/src/test/java/org/apache/shardingsphere/sharding/rewrite/parameterized/scenario/ShardingSQLRewriterParameterizedTest.java
##########
@@ -130,6 +130,7 @@ private ShardingSphereSchema mockSchema() {
when(result.get("t_account")).thenReturn(accountTableMetaData);
when(result.get("t_account_detail")).thenReturn(mock(TableMetaData.class));
when(result.getAllColumnNames("t_account")).thenReturn(Arrays.asList("account_id",
"amount", "status"));
+ when(result.containsColumn("t_account",
"account_id")).thenReturn(true);
Review comment:
@tuichenchuxin Why add this condition?
##########
File path:
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/rule/ShardingRule.java
##########
@@ -501,4 +501,15 @@ public boolean isNeedAccumulate(final Collection<String>
tables) {
private Optional<String> findActualTableFromActualDataNode(final String
catalog, final List<DataNode> actualDataNodes) {
return actualDataNodes.stream().filter(each ->
each.getDataSourceName().equalsIgnoreCase(catalog)).findFirst().map(DataNode::getTableName);
}
+
+ /**
+ * Judge logic tables is all single tables.
+ *
+ * @param logicTableNames logic table names
+ * @return logic tables is all single tables or not
+ */
+ public boolean isAllSingleTables(final Collection<String> logicTableNames)
{
Review comment:
@tuichenchuxin Please remove this method.
##########
File path:
shardingsphere-test/shardingsphere-rewrite-test/src/test/resources/scenario/sharding/case/select.xml
##########
@@ -30,7 +30,6 @@
<rewrite-assertion id="select_with_not_exists" db-type="MySQL">
<input sql="SELECT * FROM t_account a WHERE not exists (select * from
t_account_detail where a.account_id=account_id and account_id=1000) and
account_id = 100" />
<output sql="SELECT * FROM t_account_0 a WHERE not exists (select *
from t_account_detail_0 where a.account_id=account_id and account_id=1000) and
account_id = 100" />
- <output sql="SELECT * FROM t_account_1 a WHERE not exists (select *
from t_account_detail_1 where a.account_id=account_id and account_id=1000) and
account_id = 100" />
Review comment:
@tuichenchuxin Why delete this case?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
