Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support Cartesian Product for readWriteConfig, add check database for writedatasoucename #19808

Merged
merged 3 commits into from
Aug 7, 2022

Conversation

natehuangting
Copy link
Contributor

Fixes #19693 .

@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2022

Codecov Report

Merging #19808 (a1a761a) into master (2e65417) will increase coverage by 0.13%.
The diff coverage is 69.73%.

@@             Coverage Diff              @@
##             master   #19808      +/-   ##
============================================
+ Coverage     60.07%   60.20%   +0.13%     
- Complexity     2399     2423      +24     
============================================
  Files          3856     3875      +19     
  Lines         54920    55026     +106     
  Branches       7681     7699      +18     
============================================
+ Hits          32993    33130     +137     
+ Misses        19098    19051      -47     
- Partials       2829     2845      +16     
Impacted Files Coverage Δ
...config/YamlDatabaseDiscoveryRuleConfiguration.java 80.00% <ø> (ø)
...rypt/yaml/config/YamlEncryptRuleConfiguration.java 83.33% <ø> (ø)
...onfig/YamlReadwriteSplittingRuleConfiguration.java 75.00% <ø> (ø)
...hadow/yaml/config/YamlShadowRuleConfiguration.java 80.00% <ø> (ø)
...ing/data/pipeline/ShardingRuleAlteredDetector.java 0.00% <ø> (ø)
...ing/yaml/config/YamlShardingRuleConfiguration.java 90.00% <ø> (ø)
...ml/swapper/ShardingRuleConfigurationConverter.java 80.00% <ø> (ø)
.../infra/yaml/config/pojo/YamlRootConfiguration.java 0.00% <ø> (ø)
...ojo/rule/YamlOnRuleAlteredActionConfiguration.java 100.00% <ø> (ø)
.../executor/advanced/AdvancedFederationExecutor.java 30.00% <0.00%> (+0.96%) ⬆️
... and 329 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Preconditions.checkArgument(writeInlineNames.size() == inlineNames.size(), "Inline expression write data source names size error");
readInlineNames.forEach(e -> Preconditions.checkArgument(e.size() == inlineNames.size(), "Inline expression read data source names size error"));
for (int i = 0; i < inlineNames.size(); i++) {
final int finalI = i;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable does not make sense, could you rename it?

List<String> inlineNames = new InlineExpressionParser(config.getName()).splitAndEvaluate();
if (null != config.getStaticStrategy()) {
List<String> writeInlineNames = new InlineExpressionParser(config.getStaticStrategy().getWriteDataSourceName()).splitAndEvaluate();
List<List<String>> readInlineNames = config.getStaticStrategy().getReadDataSourceNames().stream().map(e -> new InlineExpressionParser(e).splitAndEvaluate()).collect(Collectors.toList());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about change readInlineNames to readInlineDataSourceNames?

@natehuangting natehuangting requested a review from terrymanu August 5, 2022 11:46
@terrymanu terrymanu added this to the 5.1.3 milestone Aug 7, 2022
@terrymanu terrymanu merged commit aa254d0 into apache:master Aug 7, 2022
@terrymanu terrymanu mentioned this pull request Aug 7, 2022
RaigorJiang pushed a commit that referenced this pull request Aug 7, 2022
* Refactor AbstractReadwriteSplittingRuleConfigurationChecker

* Refactor ReadwriteSplittingRule
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Read-write split configuration supports Cartesian product
3 participants