This is an automated email from the ASF dual-hosted git repository.
panjuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new c8adeb8271f Fix wrong index and constraints rewrite caused by single
table and broadcast table lost in RouteContext (#28181)
c8adeb8271f is described below
commit c8adeb8271f479864d5acff123e9368357bd5537
Author: Zhengqiang Duan <[email protected]>
AuthorDate: Sun Aug 20 13:15:23 2023 +0800
Fix wrong index and constraints rewrite caused by single table and
broadcast table lost in RouteContext (#28181)
---
.../broadcast/route/BroadcastSQLRouter.java | 13 +++++++++
.../complex/ShardingCartesianRoutingEngine.java | 3 +-
.../single/route/SingleSQLRouter.java | 16 +++++++---
.../single/route/SingleSQLRouterTest.java | 6 ++--
.../scenario/sharding/case/ddl/alter-table.xml | 17 +++++------
.../scenario/sharding/case/ddl/create-table.xml | 34 ++++++++++------------
6 files changed, 54 insertions(+), 35 deletions(-)
diff --git
a/features/broadcast/core/src/main/java/org/apache/shardingsphere/broadcast/route/BroadcastSQLRouter.java
b/features/broadcast/core/src/main/java/org/apache/shardingsphere/broadcast/route/BroadcastSQLRouter.java
index 236c90d1a47..eb67e3b8aa5 100644
---
a/features/broadcast/core/src/main/java/org/apache/shardingsphere/broadcast/route/BroadcastSQLRouter.java
+++
b/features/broadcast/core/src/main/java/org/apache/shardingsphere/broadcast/route/BroadcastSQLRouter.java
@@ -23,6 +23,7 @@ import org.apache.shardingsphere.broadcast.rule.BroadcastRule;
import
org.apache.shardingsphere.infra.binder.context.statement.SQLStatementContext;
import
org.apache.shardingsphere.infra.binder.context.statement.ddl.CloseStatementContext;
import org.apache.shardingsphere.infra.binder.context.type.CursorAvailable;
+import org.apache.shardingsphere.infra.binder.context.type.IndexAvailable;
import org.apache.shardingsphere.infra.binder.context.type.TableAvailable;
import org.apache.shardingsphere.infra.config.props.ConfigurationProperties;
import
org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
@@ -94,6 +95,9 @@ public final class BroadcastSQLRouter implements
SQLRouter<BroadcastRule> {
}
return;
}
+ if (sqlStatementContext instanceof IndexAvailable &&
!routeContext.getRouteUnits().isEmpty()) {
+ putAllBroadcastTables(routeContext, broadcastRule,
sqlStatementContext);
+ }
SQLStatement sqlStatement = sqlStatementContext.getSqlStatement();
boolean functionStatement = sqlStatement instanceof
CreateFunctionStatement || sqlStatement instanceof AlterFunctionStatement ||
sqlStatement instanceof DropFunctionStatement;
boolean procedureStatement = sqlStatement instanceof
CreateProcedureStatement || sqlStatement instanceof AlterProcedureStatement ||
sqlStatement instanceof DropProcedureStatement;
@@ -115,6 +119,15 @@ public final class BroadcastSQLRouter implements
SQLRouter<BroadcastRule> {
}
}
+ private static void putAllBroadcastTables(final RouteContext routeContext,
final BroadcastRule broadcastRule, final SQLStatementContext
sqlStatementContext) {
+ Collection<String> tableNames =
sqlStatementContext.getTablesContext().getTableNames();
+ for (String each :
broadcastRule.getBroadcastRuleTableNames(tableNames)) {
+ for (RouteUnit routeUnit : routeContext.getRouteUnits()) {
+ routeUnit.getTableMappers().add(new RouteMapper(each, each));
+ }
+ }
+ }
+
private static boolean isResourceGroupStatement(final SQLStatement
sqlStatement) {
// TODO add dropResourceGroupStatement, alterResourceGroupStatement
return sqlStatement instanceof MySQLCreateResourceGroupStatement ||
sqlStatement instanceof MySQLSetResourceGroupStatement;
diff --git
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/type/complex/ShardingCartesianRoutingEngine.java
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/type/complex/ShardingCartesianRoutingEngine.java
index f5bc83f2889..34876e677a0 100644
---
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/type/complex/ShardingCartesianRoutingEngine.java
+++
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/type/complex/ShardingCartesianRoutingEngine.java
@@ -31,6 +31,7 @@ import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
+import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -112,7 +113,7 @@ public final class ShardingCartesianRoutingEngine
implements ShardingRouteEngine
private Collection<RouteUnit> getRouteUnits(final String dataSource, final
Set<List<RouteMapper>> cartesianRoutingTableGroups) {
Collection<RouteUnit> result = new LinkedHashSet<>();
for (List<RouteMapper> each : cartesianRoutingTableGroups) {
- result.add(new RouteUnit(new RouteMapper(dataSource, dataSource),
each));
+ result.add(new RouteUnit(new RouteMapper(dataSource, dataSource),
new LinkedList<>(each)));
}
return result;
}
diff --git
a/kernel/single/core/src/main/java/org/apache/shardingsphere/single/route/SingleSQLRouter.java
b/kernel/single/core/src/main/java/org/apache/shardingsphere/single/route/SingleSQLRouter.java
index d6cf5845ce3..c3dbc63496e 100644
---
a/kernel/single/core/src/main/java/org/apache/shardingsphere/single/route/SingleSQLRouter.java
+++
b/kernel/single/core/src/main/java/org/apache/shardingsphere/single/route/SingleSQLRouter.java
@@ -37,7 +37,7 @@ import
org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.CreateTable
import
org.apache.shardingsphere.sql.parser.sql.common.statement.dml.DMLStatement;
import java.util.Collection;
-import java.util.Collections;
+import java.util.LinkedList;
/**
* Single SQL router.
@@ -48,7 +48,7 @@ public final class SingleSQLRouter implements
SQLRouter<SingleRule> {
public RouteContext createRouteContext(final QueryContext queryContext,
final RuleMetaData globalRuleMetaData, final ShardingSphereDatabase database,
final SingleRule rule,
final ConfigurationProperties
props, final ConnectionContext connectionContext) {
if (1 ==
database.getResourceMetaData().getStorageUnitMetaData().getStorageUnits().size())
{
- return createSingleDataSourceRouteContext(rule, database);
+ return createSingleDataSourceRouteContext(rule, database,
queryContext);
}
RouteContext result = new RouteContext();
SQLStatementContext sqlStatementContext =
queryContext.getSqlStatementContext();
@@ -80,11 +80,19 @@ public final class SingleSQLRouter implements
SQLRouter<SingleRule> {
SingleRouteEngineFactory.newInstance(singleTables,
sqlStatementContext.getSqlStatement()).ifPresent(optional ->
optional.route(routeContext, rule));
}
- private RouteContext createSingleDataSourceRouteContext(final SingleRule
rule, final ShardingSphereDatabase database) {
+ private RouteContext createSingleDataSourceRouteContext(final SingleRule
rule, final ShardingSphereDatabase database, final QueryContext queryContext) {
String logicDataSource = rule.getDataSourceNames().iterator().next();
String actualDataSource =
database.getResourceMetaData().getStorageUnitMetaData().getStorageUnits().keySet().iterator().next();
RouteContext result = new RouteContext();
- result.getRouteUnits().add(new RouteUnit(new
RouteMapper(logicDataSource, actualDataSource), Collections.emptyList()));
+ result.getRouteUnits().add(new RouteUnit(new
RouteMapper(logicDataSource, actualDataSource),
createTableMappers(queryContext.getSqlStatementContext().getTablesContext().getTableNames())));
+ return result;
+ }
+
+ private Collection<RouteMapper> createTableMappers(final
Collection<String> tableNames) {
+ Collection<RouteMapper> result = new LinkedList<>();
+ for (String each : tableNames) {
+ result.add(new RouteMapper(each, each));
+ }
return result;
}
diff --git
a/kernel/single/core/src/test/java/org/apache/shardingsphere/single/route/SingleSQLRouterTest.java
b/kernel/single/core/src/test/java/org/apache/shardingsphere/single/route/SingleSQLRouterTest.java
index 23236286ace..421483fe3a6 100644
---
a/kernel/single/core/src/test/java/org/apache/shardingsphere/single/route/SingleSQLRouterTest.java
+++
b/kernel/single/core/src/test/java/org/apache/shardingsphere/single/route/SingleSQLRouterTest.java
@@ -58,7 +58,7 @@ import java.util.Properties;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
@@ -78,7 +78,7 @@ class SingleSQLRouterTest {
RouteUnit routeUnit = actual.getRouteUnits().iterator().next();
assertThat(routeUnit.getDataSourceMapper().getLogicName(),
is("foo_ds"));
assertThat(routeUnit.getDataSourceMapper().getActualName(),
is("foo_ds"));
- assertTrue(routeUnit.getTableMappers().isEmpty());
+ assertFalse(routeUnit.getTableMappers().isEmpty());
}
private ShardingSphereDatabase mockSingleDatabase() {
@@ -99,7 +99,7 @@ class SingleSQLRouterTest {
RouteUnit routeUnit = actual.getRouteUnits().iterator().next();
assertThat(routeUnit.getDataSourceMapper().getLogicName(),
is("readwrite_ds"));
assertThat(routeUnit.getDataSourceMapper().getActualName(),
is("write_ds"));
- assertTrue(routeUnit.getTableMappers().isEmpty());
+ assertFalse(routeUnit.getTableMappers().isEmpty());
}
private ShardingSphereDatabase mockReadwriteSplittingDatabase() {
diff --git
a/test/it/rewriter/src/test/resources/scenario/sharding/case/ddl/alter-table.xml
b/test/it/rewriter/src/test/resources/scenario/sharding/case/ddl/alter-table.xml
index fc90a294f51..e21a4b457e4 100644
---
a/test/it/rewriter/src/test/resources/scenario/sharding/case/ddl/alter-table.xml
+++
b/test/it/rewriter/src/test/resources/scenario/sharding/case/ddl/alter-table.xml
@@ -61,15 +61,14 @@
<input sql="ALTER TABLE t_single ADD CONSTRAINT t_single_fk FOREIGN
KEY (order_id) REFERENCES t_order (order_id)" />
<output sql="ALTER TABLE t_single ADD CONSTRAINT t_single_fk_t_single
FOREIGN KEY (order_id) REFERENCES t_order_0 (order_id)" />
</rewrite-assertion>
- <!-- FIXME xiaoqiang -->
-<!-- <rewrite-assertion
id="alter_table_with_single_and_broadcast_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">-->
-<!-- <input sql="ALTER TABLE t_single ADD CONSTRAINT t_single_fk
FOREIGN KEY (order_id) REFERENCES t_config (order_id)" />-->
-<!-- <output sql="ALTER TABLE t_single ADD CONSTRAINT
t_single_fk_t_single FOREIGN KEY (order_id) REFERENCES t_config (order_id)"
/>-->
-<!-- </rewrite-assertion>-->
-<!-- <rewrite-assertion
id="alter_table_with_single_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">-->
-<!-- <input sql="ALTER TABLE t_single ADD CONSTRAINT t_single_fk
FOREIGN KEY (order_id) REFERENCES t_single_extend (order_id)" />-->
-<!-- <output sql="ALTER TABLE t_single ADD CONSTRAINT
t_single_fk_t_single FOREIGN KEY (order_id) REFERENCES t_single_extend
(order_id)" />-->
-<!-- </rewrite-assertion>-->
+ <rewrite-assertion
id="alter_table_with_single_and_broadcast_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">
+ <input sql="ALTER TABLE t_single ADD CONSTRAINT t_single_fk FOREIGN
KEY (order_id) REFERENCES t_config (order_id)" />
+ <output sql="ALTER TABLE t_single ADD CONSTRAINT t_single_fk_t_single
FOREIGN KEY (order_id) REFERENCES t_config (order_id)" />
+ </rewrite-assertion>
+ <rewrite-assertion
id="alter_table_with_single_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">
+ <input sql="ALTER TABLE t_single ADD CONSTRAINT t_single_fk FOREIGN
KEY (order_id) REFERENCES t_single_extend (order_id)" />
+ <output sql="ALTER TABLE t_single ADD CONSTRAINT t_single_fk_t_single
FOREIGN KEY (order_id) REFERENCES t_single_extend (order_id)" />
+ </rewrite-assertion>
<rewrite-assertion id="alter_table_with_single_table_with_rename_table"
db-types="MySQL,PostgreSQL,openGauss">
<input sql="ALTER TABLE t_single RENAME TO t_single_extend" />
<output sql="ALTER TABLE t_single RENAME TO t_single_extend" />
diff --git
a/test/it/rewriter/src/test/resources/scenario/sharding/case/ddl/create-table.xml
b/test/it/rewriter/src/test/resources/scenario/sharding/case/ddl/create-table.xml
index 1e47ac06bd4..2f52ce9119c 100644
---
a/test/it/rewriter/src/test/resources/scenario/sharding/case/ddl/create-table.xml
+++
b/test/it/rewriter/src/test/resources/scenario/sharding/case/ddl/create-table.xml
@@ -53,19 +53,18 @@
<output sql="CREATE TABLE t_account_detail_new_0(order_id INT PRIMARY
KEY, CONSTRAINT t_account_fk_t_account_detail_new_0 FOREIGN KEY (account_id)
REFERENCES t_single_new (account_id))" />
<output sql="CREATE TABLE t_account_detail_new_1(order_id INT PRIMARY
KEY, CONSTRAINT t_account_fk_t_account_detail_new_1 FOREIGN KEY (account_id)
REFERENCES t_single_new (account_id))" />
</rewrite-assertion>
- <!-- FIXME xiaoqiang -->
-<!-- <rewrite-assertion
id="create_table_with_broadcast_and_single_data_node_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">-->
-<!-- <input sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk FOREIGN KEY (order_id) REFERENCES t_order_new
(order_id))" />-->
-<!-- <output sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk_t_config_new FOREIGN KEY (order_id) REFERENCES
t_order_new_0 (order_id))" />-->
-<!-- </rewrite-assertion>-->
-<!-- <rewrite-assertion
id="create_table_with_broadcast_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">-->
-<!-- <input sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk FOREIGN KEY (order_id) REFERENCES t_order_new_type
(order_id))" />-->
-<!-- <output sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk_t_config_new FOREIGN KEY (order_id) REFERENCES
t_order_new_type (order_id))" />-->
-<!-- </rewrite-assertion>-->
-<!-- <rewrite-assertion
id="create_table_with_broadcast_and_single_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">-->
-<!-- <input sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk FOREIGN KEY (order_id) REFERENCES t_single_new
(order_id))" />-->
-<!-- <output sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk_t_config_new FOREIGN KEY (order_id) REFERENCES
t_single_new (order_id))" />-->
-<!-- </rewrite-assertion>-->
+ <rewrite-assertion
id="create_table_with_broadcast_and_single_data_node_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">
+ <input sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk FOREIGN KEY (order_id) REFERENCES t_order_new
(order_id))" />
+ <output sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk_t_config_new FOREIGN KEY (order_id) REFERENCES
t_order_new_0 (order_id))" />
+ </rewrite-assertion>
+ <rewrite-assertion
id="create_table_with_broadcast_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">
+ <input sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk FOREIGN KEY (order_id) REFERENCES t_order_new_type
(order_id))" />
+ <output sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk_t_config_new FOREIGN KEY (order_id) REFERENCES
t_order_new_type (order_id))" />
+ </rewrite-assertion>
+ <rewrite-assertion
id="create_table_with_broadcast_and_single_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">
+ <input sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk FOREIGN KEY (order_id) REFERENCES t_single_new
(order_id))" />
+ <output sql="CREATE TABLE t_config_new(order_id INT PRIMARY KEY,
CONSTRAINT t_config_new_fk_t_config_new FOREIGN KEY (order_id) REFERENCES
t_single_new (order_id))" />
+ </rewrite-assertion>
<rewrite-assertion
id="create_table_with_single_and_single_data_node_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">
<input sql="CREATE TABLE t_single_new(order_id INT PRIMARY KEY,
CONSTRAINT t_single_new_fk FOREIGN KEY (order_id) REFERENCES t_order_new
(order_id))" />
<output sql="CREATE TABLE t_single_new(order_id INT PRIMARY KEY,
CONSTRAINT t_single_new_fk FOREIGN KEY (order_id) REFERENCES t_order_new_0
(order_id))" />
@@ -75,11 +74,10 @@
<!-- <input sql="CREATE TABLE t_single_new(order_id INT PRIMARY
KEY, CONSTRAINT t_single_new_fk FOREIGN KEY (order_id) REFERENCES t_config_new
(order_id))" />-->
<!-- <output sql="CREATE TABLE t_single_new(order_id INT PRIMARY
KEY, CONSTRAINT t_single_new_fk FOREIGN KEY (order_id) REFERENCES t_config_new
(order_id))" />-->
<!-- </rewrite-assertion>-->
- <!-- FIXME xiaoqiang -->
-<!-- <rewrite-assertion
id="create_table_with_single_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">-->
-<!-- <input sql="CREATE TABLE t_single_new(order_id INT PRIMARY KEY,
CONSTRAINT t_single_new_fk FOREIGN KEY (order_id) REFERENCES t_single_extend
(order_id))" />-->
-<!-- <output sql="CREATE TABLE t_single_new(order_id INT PRIMARY KEY,
CONSTRAINT t_single_new_fk_t_single_new FOREIGN KEY (order_id) REFERENCES
t_single_extend (order_id))" />-->
-<!-- </rewrite-assertion>-->
+ <rewrite-assertion
id="create_table_with_single_table_with_add_foreign_constraint"
db-types="MySQL,PostgreSQL,openGauss">
+ <input sql="CREATE TABLE t_single_new(order_id INT PRIMARY KEY,
CONSTRAINT t_single_new_fk FOREIGN KEY (order_id) REFERENCES t_single_extend
(order_id))" />
+ <output sql="CREATE TABLE t_single_new(order_id INT PRIMARY KEY,
CONSTRAINT t_single_new_fk_t_single_new FOREIGN KEY (order_id) REFERENCES
t_single_extend (order_id))" />
+ </rewrite-assertion>
<rewrite-assertion
id="create_table_with_multi_data_node_with_storage_parameter"
db-types="openGauss">
<input sql="CREATE TABLE t_account_detail_new (order_id INT,account_id
INT) WITH (FILLFACTOR = 80, ORIENTATION=ROW)" />
<output sql="CREATE TABLE t_account_detail_new_0 (order_id
INT,account_id INT) WITH (FILLFACTOR = 80, ORIENTATION=ROW)" />
