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/09/13 10:04:24 UTC
[GitHub] [shardingsphere] tuichenchuxin opened a new pull request #12395: Refactor schema builder
tuichenchuxin opened a new pull request #12395:
URL: https://github.com/apache/shardingsphere/pull/12395
Fixes #12113.
Changes proposed in this pull request:
- refactor schema builder
--
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] strongduanmu commented on a change in pull request #12395: Refactor schema builder
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #12395:
URL: https://github.com/apache/shardingsphere/pull/12395#discussion_r708001829
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/metadata/ShardingTableMetaDataBuilder.java
##########
@@ -62,10 +62,10 @@
return Collections.emptyMap();
}
Collection<TableMetaData> tableMetaDatas = TableMetaDataLoaderEngine.load(tableMetaDataLoaderMaterials, materials.getDatabaseType());
- return isCheckingMetaData ? decorateWithCheckTableMetaData(tableMetaDatas, rule) : decorateLogicTableName(tableMetaDatas, rule);
+ return isCheckingMetaData ? getTableMetaDataMapWithCheckTableMetaData(tableMetaDatas, rule) : getTableMetaDataMap(tableMetaDatas, rule);
Review comment:
@tuichenchuxin Can we split check and getTableMetaDataMap logic?
--
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] strongduanmu commented on a change in pull request #12395: Refactor schema builder
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #12395:
URL: https://github.com/apache/shardingsphere/pull/12395#discussion_r707830695
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -38,45 +37,38 @@
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public final class SchemaBuilder {
- static {
- ShardingSphereServiceLoader.register(DialectTableMetaDataLoader.class);
- }
-
/**
- * build actual and logic table meta data.
+ * Build sharding sphere schema.
*
* @param materials schema builder materials
- * @return actual and logic table meta data
+ * @return sharding sphere schema
* @throws SQLException SQL exception
*/
- public static Map<TableMetaData, TableMetaData> build(final SchemaBuilderMaterials materials) throws SQLException {
- Map<String, TableMetaData> actualTableMetaMap = buildActualTableMetaDataMap(materials);
- Map<String, TableMetaData> logicTableMetaMap = buildLogicTableMetaDataMap(materials, actualTableMetaMap);
- Map<TableMetaData, TableMetaData> result = new HashMap<>(actualTableMetaMap.size(), 1);
- for (Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
- result.put(entry.getValue(), logicTableMetaMap.getOrDefault(entry.getKey(), entry.getValue()));
- }
- return result;
- }
-
- private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
+ public static ShardingSphereSchema build(final SchemaBuilderMaterials materials) throws SQLException {
Collection<String> allTableNames = materials.getRules().stream().filter(each -> each instanceof TableContainedRule)
.flatMap(shardingSphereRule -> ((TableContainedRule) shardingSphereRule).getTables().stream()).collect(Collectors.toSet());
- return TableMetaDataBuilder.load(allTableNames, materials);
+ return new ShardingSphereSchema(TableMetaDataBuilder.load(allTableNames, materials));
}
- private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) {
- Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
+ /**
+ * Decorate sharding sphere schema.
+ *
+ * @param schema sharding sphere schema
+ * @param materials schema builder materials
+ * @return sharding sphere schema
+ */
+ public static ShardingSphereSchema decorateSchema(final ShardingSphereSchema schema, final SchemaBuilderMaterials materials) {
Review comment:
@tuichenchuxin `decorate` may be better.
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -38,45 +37,38 @@
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public final class SchemaBuilder {
- static {
- ShardingSphereServiceLoader.register(DialectTableMetaDataLoader.class);
- }
-
/**
- * build actual and logic table meta data.
+ * Build sharding sphere schema.
*
* @param materials schema builder materials
- * @return actual and logic table meta data
+ * @return sharding sphere schema
* @throws SQLException SQL exception
*/
- public static Map<TableMetaData, TableMetaData> build(final SchemaBuilderMaterials materials) throws SQLException {
- Map<String, TableMetaData> actualTableMetaMap = buildActualTableMetaDataMap(materials);
- Map<String, TableMetaData> logicTableMetaMap = buildLogicTableMetaDataMap(materials, actualTableMetaMap);
- Map<TableMetaData, TableMetaData> result = new HashMap<>(actualTableMetaMap.size(), 1);
- for (Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
- result.put(entry.getValue(), logicTableMetaMap.getOrDefault(entry.getKey(), entry.getValue()));
- }
- return result;
- }
-
- private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
+ public static ShardingSphereSchema build(final SchemaBuilderMaterials materials) throws SQLException {
Collection<String> allTableNames = materials.getRules().stream().filter(each -> each instanceof TableContainedRule)
.flatMap(shardingSphereRule -> ((TableContainedRule) shardingSphereRule).getTables().stream()).collect(Collectors.toSet());
- return TableMetaDataBuilder.load(allTableNames, materials);
+ return new ShardingSphereSchema(TableMetaDataBuilder.load(allTableNames, materials));
}
- private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) {
- Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
+ /**
+ * Decorate sharding sphere schema.
+ *
+ * @param schema sharding sphere schema
+ * @param materials schema builder materials
+ * @return sharding sphere schema
+ */
+ public static ShardingSphereSchema decorateSchema(final ShardingSphereSchema schema, final SchemaBuilderMaterials materials) {
+ Map<String, TableMetaData> tableMetaDataMap = new LinkedHashMap<>(schema.getTables());
for (ShardingSphereRule rule : materials.getRules()) {
if (rule instanceof TableContainedRule) {
for (String table : ((TableContainedRule) rule).getTables()) {
- if (tables.containsKey(table)) {
- TableMetaData metaData = TableMetaDataBuilder.decorate(table, tables.get(table), materials.getRules());
- result.put(table, metaData);
+ if (tableMetaDataMap.containsKey(table.toLowerCase())) {
Review comment:
@tuichenchuxin Can we remove toLowerCase logic? We can get the actual `TableMetaData` from schema.
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/loader/universal/DefaultTableMetaDataLoader.java
##########
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-package org.apache.shardingsphere.infra.metadata.schema.builder.loader;
+package org.apache.shardingsphere.infra.metadata.schema.builder.loader.universal;
Review comment:
@tuichenchuxin Do you think `common` is better?
--
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 #12395: Refactor schema builder
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #12395:
URL: https://github.com/apache/shardingsphere/pull/12395#issuecomment-918747585
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/12395?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 [#12395](https://codecov.io/gh/apache/shardingsphere/pull/12395?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a0e1331) into [master](https://codecov.io/gh/apache/shardingsphere/commit/b90eb9b5f22286794e3f5665821a498b5ebe3554?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b90eb9b) will **decrease** coverage by `0.02%`.
> The diff coverage is `57.14%`.
[](https://codecov.io/gh/apache/shardingsphere/pull/12395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #12395 +/- ##
============================================
- Coverage 63.73% 63.71% -0.03%
- Complexity 1293 1295 +2
============================================
Files 2380 2380
Lines 36212 36201 -11
Branches 6292 6291 -1
============================================
- Hits 23081 23065 -16
- Misses 11270 11277 +7
+ Partials 1861 1859 -2
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/12395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../encrypt/metadata/EncryptTableMetaDataBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/12395/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VuY3J5cHQvbWV0YWRhdGEvRW5jcnlwdFRhYmxlTWV0YURhdGFCdWlsZGVyLmphdmE=) | `88.88% <ø> (ø)` | |
| [...ma/builder/loader/common/ColumnMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/12395/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvY29tbW9uL0NvbHVtbk1ldGFEYXRhTG9hZGVyLmphdmE=) | `87.50% <ø> (ø)` | |
| [...ema/builder/loader/common/IndexMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/12395/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvY29tbW9uL0luZGV4TWV0YURhdGFMb2FkZXIuamF2YQ==) | `66.66% <ø> (ø)` | |
| [...ema/builder/loader/common/TableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/12395/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvY29tbW9uL1RhYmxlTWV0YURhdGFMb2FkZXIuamF2YQ==) | `80.00% <ø> (ø)` | |
| [...etadata/schema/builder/util/TableMetaDataUtil.java](https://codecov.io/gh/apache/shardingsphere/pull/12395/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci91dGlsL1RhYmxlTWV0YURhdGFVdGlsLmphdmE=) | `0.00% <ø> (ø)` | |
| [...ngletable/metadata/SingleTableMetaDataBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/12395/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLXNpbmdsZS10YWJsZS9zaGFyZGluZ3NwaGVyZS1zaW5nbGUtdGFibGUtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2luZ2xldGFibGUvbWV0YWRhdGEvU2luZ2xlVGFibGVNZXRhRGF0YUJ1aWxkZXIuamF2YQ==) | `61.53% <ø> (ø)` | |
| [...caling/core/common/datasource/MetaDataManager.java](https://codecov.io/gh/apache/shardingsphere/pull/12395/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-c2hhcmRpbmdzcGhlcmUtc2NhbGluZy9zaGFyZGluZ3NwaGVyZS1zY2FsaW5nLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NjYWxpbmcvY29yZS9jb21tb24vZGF0YXNvdXJjZS9NZXRhRGF0YU1hbmFnZXIuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...e/infra/metadata/schema/builder/SchemaBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/12395/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9TY2hlbWFCdWlsZGVyLmphdmE=) | `21.42% <14.28%> (-69.49%)` | :arrow_down: |
| [...harding/metadata/ShardingTableMetaDataBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/12395/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvbWV0YWRhdGEvU2hhcmRpbmdUYWJsZU1ldGFEYXRhQnVpbGRlci5qYXZh) | `56.92% <100.00%> (ø)` | |
| [.../metadata/schema/builder/TableMetaDataBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/12395/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9UYWJsZU1ldGFEYXRhQnVpbGRlci5qYXZh) | `86.36% <100.00%> (ø)` | |
| ... and [4 more](https://codecov.io/gh/apache/shardingsphere/pull/12395/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/12395?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/12395?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 [f946aef...a0e1331](https://codecov.io/gh/apache/shardingsphere/pull/12395?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] strongduanmu merged pull request #12395: Refactor schema builder
Posted by GitBox <gi...@apache.org>.
strongduanmu merged pull request #12395:
URL: https://github.com/apache/shardingsphere/pull/12395
--
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