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 2022/05/28 15:03:35 UTC

[GitHub] [shardingsphere] azexcy opened a new pull request, #18035: Refactoring table schema relation in scaling, replace TableNameSchemaNameMapping

azexcy opened a new pull request, #18035:
URL: https://github.com/apache/shardingsphere/pull/18035

   
   Changes proposed in this pull request:
   - Refactoring table schema relation.
   - remove TableNameSchemaNameMapping


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] codecov-commenter commented on pull request #18035: Refactoring table schema relation in scaling, replace TableNameSchemaNameMapping

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #18035:
URL: https://github.com/apache/shardingsphere/pull/18035#issuecomment-1140285472

   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/18035?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18035](https://codecov.io/gh/apache/shardingsphere/pull/18035?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e32b8da) into [master](https://codecov.io/gh/apache/shardingsphere/commit/df10532d7010b83ab5dbab0d9463b2fc4bb775ed?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (df10532) will **decrease** coverage by `0.00%`.
   > The diff coverage is `3.44%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #18035      +/-   ##
   ============================================
   - Coverage     58.94%   58.93%   -0.01%     
     Complexity     2146     2146              
   ============================================
     Files          3654     3653       -1     
     Lines         54108    54115       +7     
     Branches       9192     9196       +4     
   ============================================
   + Hits          31893    31894       +1     
   - Misses        19511    19517       +6     
     Partials       2704     2704              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/18035?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...e/ShardingRuleAlteredJobConfigurationPreparer.java](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvZGF0YS9waXBlbGluZS9TaGFyZGluZ1J1bGVBbHRlcmVkSm9iQ29uZmlndXJhdGlvblByZXBhcmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...ipeline/api/config/ingest/DumperConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvYXBpL2NvbmZpZy9pbmdlc3QvRHVtcGVyQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...pi/config/ingest/InventoryDumperConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvYXBpL2NvbmZpZy9pbmdlc3QvSW52ZW50b3J5RHVtcGVyQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [.../api/config/rulealtered/ImporterConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvYXBpL2NvbmZpZy9ydWxlYWx0ZXJlZC9JbXBvcnRlckNvbmZpZ3VyYXRpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...onfig/rulealtered/RuleAlteredJobConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvYXBpL2NvbmZpZy9ydWxlYWx0ZXJlZC9SdWxlQWx0ZXJlZEpvYkNvbmZpZ3VyYXRpb24uamF2YQ==) | `75.00% <ø> (ø)` | |
   | [...ealtered/yaml/YamlRuleAlteredJobConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvYXBpL2NvbmZpZy9ydWxlYWx0ZXJlZC95YW1sL1lhbWxSdWxlQWx0ZXJlZEpvYkNvbmZpZ3VyYXRpb24uamF2YQ==) | `11.76% <ø> (ø)` | |
   | [...core/check/consistency/DataConsistencyChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvY2hlY2svY29uc2lzdGVuY3kvRGF0YUNvbnNpc3RlbmN5Q2hlY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...hm/DataMatchDataConsistencyCalculateAlgorithm.java](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvY2hlY2svY29uc2lzdGVuY3kvYWxnb3JpdGhtL0RhdGFNYXRjaERhdGFDb25zaXN0ZW5jeUNhbGN1bGF0ZUFsZ29yaXRobS5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...re/check/datasource/AbstractDataSourceChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvY2hlY2svZGF0YXNvdXJjZS9BYnN0cmFjdERhdGFTb3VyY2VDaGVja2VyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...prepare/datasource/AbstractDataSourcePreparer.java](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvcHJlcGFyZS9kYXRhc291cmNlL0Fic3RyYWN0RGF0YVNvdXJjZVByZXBhcmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | ... and [9 more](https://codecov.io/gh/apache/shardingsphere/pull/18035/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/18035?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/18035?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [df10532...e32b8da](https://codecov.io/gh/apache/shardingsphere/pull/18035?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] sandynz closed pull request #18035: Refactoring table schema relation in scaling, replace TableNameSchemaNameMapping

Posted by GitBox <gi...@apache.org>.
sandynz closed pull request #18035: Refactoring table schema relation in scaling, replace TableNameSchemaNameMapping
URL: https://github.com/apache/shardingsphere/pull/18035


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] sandynz commented on a diff in pull request #18035: Refactoring table schema relation in scaling, replace TableNameSchemaNameMapping

Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #18035:
URL: https://github.com/apache/shardingsphere/pull/18035#discussion_r884196854


##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/data/pipeline/ShardingRuleAlteredJobConfigurationPreparer.java:
##########
@@ -75,10 +75,31 @@ public final class ShardingRuleAlteredJobConfigurationPreparer implements RuleAl
     
     @Override
     public void extendJobConfiguration(final YamlRuleAlteredJobConfiguration yamlJobConfig) {
-        Map<String, List<DataNode>> actualDataNodes = getActualDataNodes(new RuleAlteredJobConfigurationSwapper().swapToObject(yamlJobConfig));
+        RuleAlteredJobConfiguration jobConfig = new RuleAlteredJobConfigurationSwapper().swapToObject(yamlJobConfig);
+        Map<String, List<DataNode>> actualDataNodes = getActualDataNodes(jobConfig);
         yamlJobConfig.setJobShardingDataNodes(getJobShardingDataNodes(actualDataNodes));
         yamlJobConfig.setLogicTables(getLogicTables(actualDataNodes.keySet()));
         yamlJobConfig.setTablesFirstDataNodes(getTablesFirstDataNodes(actualDataNodes));
+        ShardingSphereDatabase database = PipelineContext.getContextManager().getMetaDataContexts().getMetaData().getDatabases().get(jobConfig.getDatabaseName());
+        Map<String, String> tableSchema = new HashMap<>(actualDataNodes.size(), 1);
+        for (String logicTable : actualDataNodes.keySet()) {
+            tableSchema.put(logicTable, getLogicTableSchema(database.getSchemas(), new LogicTableName(logicTable)));
+        }
+        yamlJobConfig.setSourceTableSchemaMap(tableSchema);
+        yamlJobConfig.setTargetTableSchemaMap(tableSchema);
+    }
+    
+    private static String getLogicTableSchema(final Map<String, ShardingSphereSchema> schemaMap, final LogicTableName logicTableName) {
+        if (schemaMap == null || schemaMap.isEmpty()) {
+            return null;
+        }
+        // TODO if create same table, will use the first, may cause problem.
+        for (Entry<String, ShardingSphereSchema> entry : schemaMap.entrySet()) {
+            if (entry.getValue().containsTable(logicTableName.getLowercase())) {
+                return entry.getKey();
+            }
+        }
+        return null;
     }

Review Comment:
   Seems it still get first schema name by logic table name.



##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/data/pipeline/ShardingRuleAlteredJobConfigurationPreparer.java:
##########
@@ -75,10 +75,31 @@ public final class ShardingRuleAlteredJobConfigurationPreparer implements RuleAl
     
     @Override
     public void extendJobConfiguration(final YamlRuleAlteredJobConfiguration yamlJobConfig) {
-        Map<String, List<DataNode>> actualDataNodes = getActualDataNodes(new RuleAlteredJobConfigurationSwapper().swapToObject(yamlJobConfig));
+        RuleAlteredJobConfiguration jobConfig = new RuleAlteredJobConfigurationSwapper().swapToObject(yamlJobConfig);
+        Map<String, List<DataNode>> actualDataNodes = getActualDataNodes(jobConfig);
         yamlJobConfig.setJobShardingDataNodes(getJobShardingDataNodes(actualDataNodes));
         yamlJobConfig.setLogicTables(getLogicTables(actualDataNodes.keySet()));
         yamlJobConfig.setTablesFirstDataNodes(getTablesFirstDataNodes(actualDataNodes));
+        ShardingSphereDatabase database = PipelineContext.getContextManager().getMetaDataContexts().getMetaData().getDatabases().get(jobConfig.getDatabaseName());
+        Map<String, String> tableSchema = new HashMap<>(actualDataNodes.size(), 1);
+        for (String logicTable : actualDataNodes.keySet()) {
+            tableSchema.put(logicTable, getLogicTableSchema(database.getSchemas(), new LogicTableName(logicTable)));
+        }
+        yamlJobConfig.setSourceTableSchemaMap(tableSchema);
+        yamlJobConfig.setTargetTableSchemaMap(tableSchema);

Review Comment:
   For common fields, it could be put in `RuleAlteredJobWorker.createJobConfig`. `extendJobConfiguration` is used to extend special fields in SPI impl.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] sandynz commented on pull request #18035: Refactoring table schema relation in scaling, replace TableNameSchemaNameMapping

Posted by GitBox <gi...@apache.org>.
sandynz commented on PR #18035:
URL: https://github.com/apache/shardingsphere/pull/18035#issuecomment-1144439758

   Close for now, we might change job configuration structure to support it integrally


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org