keith-turner commented on code in PR #5990:
URL: https://github.com/apache/accumulo/pull/5990#discussion_r2578238927
##########
core/src/main/java/org/apache/accumulo/core/client/admin/NewTableConfiguration.java:
##########
@@ -167,7 +188,8 @@ public NewTableConfiguration
setProperties(Map<String,String> props) {
public Map<String,String> getProperties() {
Map<String,String> propertyMap = new HashMap<>();
- if (limitVersion) {
+ if (includeDefaults) {
+ checkIteratorConflictsWithDefaults();
Review Comment:
This code does not check the for conflicts with the default constraints. It
also does not check for conflicts with iterators set via properties instead of
the attachIter method. Could do something like the following after everything
is added to `propertyMap` and look for default props with different values. We
could ignore cases where the user set the default prop w/ the same value (that
may avoid errors w/ some current cases of copying existing config).
```java
if (includeDefaults) {
var defaults = IteratorConfigUtil.getInitialTableProperties();
defaults.forEach((dk,dv)->{
var value = propertyMap.get(dk);
if(value != null && !value.equals(dv)) {
throw new IllegalStateException("For property "+dk+" value
conflicts with default : "+dv+" != "+value);
}
});
propertyMap.putAll(defaults);
}
```
One problem with this vs the current code in this PR is that its checks for
the iterators are not as strong.
--
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]
