sandynz commented on code in PR #20207:
URL: https://github.com/apache/shardingsphere/pull/20207#discussion_r946284434


##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/metadata/generator/PipelineDDLGenerator.java:
##########
@@ -67,27 +65,31 @@ public final class PipelineDDLGenerator {
     private static final String DELIMITER = ";";
     
     private static final String SET_SEARCH_PATH_PREFIX = "set search_path";
-    
-    private final ContextManager contextManager;
-    
+
     /**
      * Generate logic ddl sql.
      *
-     * @param databaseType database type
+     * @param dataSource data source
      * @param databaseName database name
      * @param schemaName schema name
-     * @param tableName table name
+     * @param logicTableName table name
+     * @param actualTableName actual table name
+     * @param databases databases
+     * @param parserEngine parser engine
      * @return ddl SQL
      */
     @SneakyThrows
-    public String generateLogicDDLSQL(final DatabaseType databaseType, final 
String databaseName, final String schemaName, final String tableName) {
-        ShardingSphereDatabase database = 
contextManager.getMetaDataContexts().getMetaData().getDatabase(databaseName);
+    public String generateLogicDDLSQL(final DataSource dataSource, final 
String databaseName, final String schemaName,
+                                      final String logicTableName, final 
String actualTableName,
+                                      final Map<String, 
ShardingSphereDatabase> databases, final ShardingSphereSQLParserEngine 
parserEngine) {

Review Comment:
   Could we remove `Map<String, ShardingSphereDatabase> databases` parameter?
   `SQLStatementContextFactory.newInstance` doesn't need databases for 
DDLStatement.
   And other needed fields could be put in parameters.



##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-dialect/shardingsphere-data-pipeline-mysql/src/main/java/org/apache/shardingsphere/data/pipeline/mysql/prepare/datasource/MySQLDataSourcePreparer.java:
##########
@@ -52,11 +57,19 @@ public void prepareTargetTables(final 
PrepareTargetTablesParameter parameter) {
     }
     
     private List<String> getCreateTableSQL(final PrepareTargetTablesParameter 
parameter) {
-        PipelineDDLGenerator generator = new 
PipelineDDLGenerator(PipelineContext.getContextManager());
+        PipelineDDLGenerator generator = new PipelineDDLGenerator();
+        DataSource dataSource = 
parameter.getDataSourceManager().getDataSource(parameter.getDataSourceConfig());
+        ShardingSphereMetaData metaData = 
PipelineContext.getContextManager().getMetaDataContexts().getMetaData();

Review Comment:
   `DataSource dataSource = 
parameter.getDataSourceManager().getDataSource(parameter.getDataSourceConfig());`
 might return ShardingSphereDataSource, could we reference the original code 
`database.getResource().getDataSources().get(dataSourceName)` in 
PipelineDDLGenerator?
   
   And also PostgreSQL and openGauss implementations.
   
   You could refer to:
   - PrepareTargetTablesParameter.tablesFirstDataNodes
   - JobDataNodeLine, JobDataNodeEntry (logicTableName, dataNodes)
   - DataNode (dataSourceName, tableName), e.g. ds_0.t_order_0
   
   



-- 
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]

Reply via email to