You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/06/16 08:34:12 UTC
[GitHub] [incubator-doris] pengxiangyu opened a new pull request, #10190: Fix duplicate code for PropertyAnalyzer.analyzeDataProperty()
pengxiangyu opened a new pull request, #10190:
URL: https://github.com/apache/incubator-doris/pull/10190
# Proposed changes
Issue Number: close #10189
## Problem Summary:
PropertyAnalyzer.analyzeDataProperty(Map<String, String> properties, final DataProperty oldDataProperty) has something not suitable.
Parameter oldDataProperty is the old DataProperty, properties should be used to replace some of its members.
If properties has no some members, old ones need to be left, but not be set to default value.
Function modifyPartitionsProperty() uses analyzeDataProperty(), but create a new DataProperty again, it is duplicate.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman merged pull request #10190: Fix duplicate code for PropertyAnalyzer.analyzeDataProperty()
Posted by GitBox <gi...@apache.org>.
morningman merged PR #10190:
URL: https://github.com/apache/doris/pull/10190
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10190: Fix duplicate code for PropertyAnalyzer.analyzeDataProperty()
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10190:
URL: https://github.com/apache/doris/pull/10190#issuecomment-1159964585
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a diff in pull request #10190: Fix duplicate code for PropertyAnalyzer.analyzeDataProperty()
Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #10190:
URL: https://github.com/apache/incubator-doris/pull/10190#discussion_r898972365
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java:
##########
@@ -109,16 +109,23 @@ public class PropertyAnalyzer {
public static final String PROPERTIES_DISABLE_LOAD = "disable_load";
- public static DataProperty analyzeDataProperty(Map<String, String> properties, DataProperty oldDataProperty)
+ /**
+ * check and replace members of DataProperty by properties.
+ * @param properties key->value for members to change.
+ * @param oldDataProperty old DataProperty
+ * @return new DataProperty
+ * @throws AnalysisException property has invalid key->value
+ */
+ public static DataProperty analyzeDataProperty(Map<String, String> properties, final DataProperty oldDataProperty)
throws AnalysisException {
if (properties == null || properties.isEmpty()) {
return oldDataProperty;
}
- TStorageMedium storageMedium = null;
- long cooldownTimeStamp = DataProperty.MAX_COOLDOWN_TIME_MS;
- String remoteStorageResourceName = "";
- long remoteCooldownTimeStamp = DataProperty.MAX_COOLDOWN_TIME_MS;
+ TStorageMedium storageMedium = oldDataProperty.getStorageMedium();
+ long cooldownTimeStamp = oldDataProperty.getCooldownTimeMs();
+ String remoteStorageResourceName = oldDataProperty.getRemoteStorageResourceName();
+ long remoteCooldownTimeStamp = oldDataProperty.getRemoteCooldownTimeMs();
Review Comment:
Should we change the logic in line 161?
```
if (!hasCooldown && !hasMedium) {
return oldDataProperty;
}
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10190: Fix duplicate code for PropertyAnalyzer.analyzeDataProperty()
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10190:
URL: https://github.com/apache/doris/pull/10190#issuecomment-1159964553
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org