You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2021/04/27 10:08:19 UTC
[GitHub] [shardingsphere] tristaZero commented on a change in pull request #10206: Load metadata for tables without table rule
tristaZero commented on a change in pull request #10206:
URL: https://github.com/apache/shardingsphere/pull/10206#discussion_r621064913
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/metadata/schema/refresher/type/CreateTableStatementSchemaRefresherTest.java
##########
@@ -31,90 +36,79 @@
import org.apache.shardingsphere.sql.parser.sql.dialect.statement.sqlserver.ddl.SQLServerCreateTableStatement;
import org.junit.Test;
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
public final class CreateTableStatementSchemaRefresherTest {
+ private SchemaBuilderMaterials materials = mock(SchemaBuilderMaterials.class);
+
@Test
public void refreshForMySQL() throws SQLException {
MySQLCreateTableStatement createTableStatement = new MySQLCreateTableStatement();
createTableStatement.setNotExisted(false);
+ when(materials.getDatabaseType()).thenReturn(new MySQLDatabaseType());
refresh(createTableStatement);
}
@Test
public void refreshForOracle() throws SQLException {
OracleCreateTableStatement createTableStatement = new OracleCreateTableStatement();
+ when(materials.getDatabaseType()).thenReturn(new OracleDatabaseType());
refresh(createTableStatement);
}
@Test
public void refreshForPostgreSQL() throws SQLException {
PostgreSQLCreateTableStatement createTableStatement = new PostgreSQLCreateTableStatement();
createTableStatement.setNotExisted(false);
+ when(materials.getDatabaseType()).thenReturn(new PostgreSQLDatabaseType());
refresh(createTableStatement);
}
@Test
public void refreshForSQL92() throws SQLException {
SQL92CreateTableStatement createTableStatement = new SQL92CreateTableStatement();
+ when(materials.getDatabaseType()).thenReturn(new SQL92DatabaseType());
refresh(createTableStatement);
}
@Test
public void refreshForSQLServer() throws SQLException {
SQLServerCreateTableStatement createTableStatement = new SQLServerCreateTableStatement();
+ when(materials.getDatabaseType()).thenReturn(new SQLServerDatabaseType());
refresh(createTableStatement);
}
+ // TODO add more tests for tables with table rule
private void refresh(final CreateTableStatement createTableStatement) throws SQLException {
- ShardingSphereSchema schema = ShardingSphereSchemaBuildUtil.buildSchema();
createTableStatement.setTable(new SimpleTableSegment(new TableNameSegment(1, 3, new IdentifierValue("t_order_0"))));
+ Map<String, DataSource> dataSourceMap = mock(HashMap.class);
+ when(materials.getDataSourceMap()).thenReturn(dataSourceMap);
+ DataSource dataSource = mock(DataSource.class);
+ when(dataSourceMap.get(eq("ds"))).thenReturn(dataSource);
+ Connection connection = mock(Connection.class);
+ when(dataSource.getConnection()).thenReturn(connection);
+ DatabaseMetaData metaData = mock(DatabaseMetaData.class);
+ when(connection.getMetaData()).thenReturn(metaData);
+ ResultSet resultSet = mock(ResultSet.class);
+ when(metaData.getTables(any(), any(), any(), any())).thenReturn(resultSet);
+ when(resultSet.next()).thenReturn(false);
+ ShardingSphereSchema schema = ShardingSphereSchemaBuildUtil.buildSchema();
SchemaRefresher<CreateTableStatement> schemaRefresher = new CreateTableStatementSchemaRefresher();
- SchemaBuilderMaterials materials = mock(SchemaBuilderMaterials.class);
schemaRefresher.refresh(schema, Collections.singleton("ds"), createTableStatement, materials);
assertTrue(schema.containsTable("t_order_0"));
}
-
- @Test
- public void refreshWithUnConfiguredForMySQL() throws SQLException {
- MySQLCreateTableStatement createTableStatement = new MySQLCreateTableStatement();
- createTableStatement.setNotExisted(false);
- refreshWithUnConfigured(createTableStatement);
- }
-
- @Test
- public void refreshWithUnConfiguredForOracle() throws SQLException {
- OracleCreateTableStatement createTableStatement = new OracleCreateTableStatement();
- refreshWithUnConfigured(createTableStatement);
- }
-
- @Test
- public void refreshWithUnConfiguredForPostgreSQL() throws SQLException {
- PostgreSQLCreateTableStatement createTableStatement = new PostgreSQLCreateTableStatement();
- createTableStatement.setNotExisted(false);
- refreshWithUnConfigured(createTableStatement);
- }
-
- @Test
- public void refreshWithUnConfiguredForSQL92() throws SQLException {
- SQL92CreateTableStatement createTableStatement = new SQL92CreateTableStatement();
- refreshWithUnConfigured(createTableStatement);
- }
-
- @Test
- public void refreshWithUnConfiguredForSQLServer() throws SQLException {
- SQLServerCreateTableStatement createTableStatement = new SQLServerCreateTableStatement();
Review comment:
Do we add a UT to cover the branch of `!containsInTableContainedRule(tableName, materials)` ?
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/refresher/type/AlterTableStatementSchemaRefresher.java
##########
@@ -38,7 +39,8 @@ public void refresh(final ShardingSphereSchema schema, final Collection<String>
final AlterTableStatement sqlStatement, final SchemaBuilderMaterials materials) throws SQLException {
String tableName = sqlStatement.getTable().getTableName().getIdentifier().getValue();
if (!containsInTableContainedRule(tableName, materials)) {
- return;
+ TableMetaDataLoader.load(materials.getDataSourceMap().get(routeDataSourceNames.iterator().next()),
+ tableName, materials.getDatabaseType()).ifPresent(tableMetaData -> schema.put(tableName, tableMetaData));
}
if (null != schema && schema.containsTable(tableName)) {
TableMetaDataBuilder.build(tableName, materials).ifPresent(tableMetaData -> schema.put(tableName, tableMetaData));
Review comment:
Do you think `if() {} else if() {}` is more appropriate?
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/refresher/type/CreateTableStatementSchemaRefresher.java
##########
@@ -44,7 +45,7 @@ public void refresh(final ShardingSphereSchema schema, final Collection<String>
if (containsInTableContainedRule(tableName, materials)) {
tableMetaData = TableMetaDataBuilder.build(tableName, materials).orElse(new TableMetaData());
} else {
- tableMetaData = new TableMetaData();
+ tableMetaData = TableMetaDataLoader.load(materials.getDataSourceMap().get(routeDataSourceNames.iterator().next()), tableName, materials.getDatabaseType()).orElse(new TableMetaData());
Review comment:
Is it necessary to check `routeDataSourceNames.size()`
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/refresher/type/AlterTableStatementSchemaRefresher.java
##########
@@ -38,7 +39,8 @@ public void refresh(final ShardingSphereSchema schema, final Collection<String>
final AlterTableStatement sqlStatement, final SchemaBuilderMaterials materials) throws SQLException {
String tableName = sqlStatement.getTable().getTableName().getIdentifier().getValue();
if (!containsInTableContainedRule(tableName, materials)) {
- return;
+ TableMetaDataLoader.load(materials.getDataSourceMap().get(routeDataSourceNames.iterator().next()),
+ tableName, materials.getDatabaseType()).ifPresent(tableMetaData -> schema.put(tableName, tableMetaData));
}
if (null != schema && schema.containsTable(tableName)) {
TableMetaDataBuilder.build(tableName, materials).ifPresent(tableMetaData -> schema.put(tableName, tableMetaData));
Review comment:
Beside, we still need to give a check on `routeDataSourceNames.size()`
--
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:
users@infra.apache.org