kevinrr888 commented on code in PR #5990:
URL: https://github.com/apache/accumulo/pull/5990#discussion_r2557899127
##########
core/src/main/java/org/apache/accumulo/core/client/admin/NewTableConfiguration.java:
##########
@@ -168,7 +168,7 @@ public Map<String,String> getProperties() {
Map<String,String> propertyMap = new HashMap<>();
if (limitVersion) {
-
propertyMap.putAll(IteratorConfigUtil.generateInitialTableProperties(limitVersion));
+ propertyMap.putAll(IteratorConfigUtil.getInitialTableProperties());
}
Review Comment:
Currently also adds DefaultKeySizeConstraint, is this intentionally only
added if limitVersion (i.e., if `withoutDefaultIterators()` is not set)
##########
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java:
##########
@@ -454,4 +456,72 @@ public void getTimeTypeTest() throws
TableNotFoundException, AccumuloException,
"specified table does not exist");
}
+ @Test
+ public void testTablePropsEqual() throws Exception {
+ // Want to ensure users can somehow create a table via the Java API that
has exactly
+ // the same properties of another table (including changes to default
properties), similar to
+ // the copy config option of the create table Shell command. Cloning the
table is expected to
+ // be one way and modifying the properties after creation is expected to
be another way.
Review Comment:
Wanted to test the Java API equivalent to --copy-config but doesn't seem
like there is one. NewTableConfiguration doesn't appear to have any such
functionality (closest is setProperties() but that isn't quite the same), and
no TableOperation to create the table with same config as another table. Best
way I could think of accomplishing --copy-config via the Java API is modifying
the props after the table is created or clone (but obviously clone does more
than just copy the config).
##########
shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java:
##########
@@ -150,29 +152,30 @@ public int execute(final String fullCommand, final
CommandLine cl, final Shell s
}
// Copy configuration options if flag was set
+ Map<String,String> srcTableConfig = null;
if (cl.hasOption(createTableOptCopyConfig.getOpt())) {
String srcTable = cl.getOptionValue(createTableOptCopyConfig.getOpt());
if (cl.hasOption(createTableOptExcludeParentProps.getLongOpt())) {
// copy properties, excludes parent properties in configuration
- Map<String,String> tableProps =
+ srcTableConfig =
shellState.getAccumuloClient().tableOperations().getTableProperties(srcTable);
- tableProps.entrySet().stream()
- .filter(entry -> Property.isValidTablePropertyKey(entry.getKey()))
- .forEach(entry -> initProperties.put(entry.getKey(),
entry.getValue()));
} else {
// copy configuration (include parent properties)
- final Map<String,String> configuration =
+ srcTableConfig =
shellState.getAccumuloClient().tableOperations().getConfiguration(srcTable);
- configuration.entrySet().stream()
- .filter(entry -> Property.isValidTablePropertyKey(entry.getKey()))
- .forEach(entry -> initProperties.put(entry.getKey(),
entry.getValue()));
}
+ srcTableConfig.entrySet().stream()
+ .filter(entry -> Property.isValidTablePropertyKey(entry.getKey()))
+ .forEach(entry -> initProperties.put(entry.getKey(),
entry.getValue()));
}
// if no defaults selected, remove, even if copied from configuration or
properties
if (cl.hasOption(createTableNoDefaultIters.getOpt())) {
- Set<String> initialProps =
IteratorConfigUtil.generateInitialTableProperties(true).keySet();
+ // handles if default props were copied over
+ Set<String> initialProps =
IteratorConfigUtil.getInitialTableProperties().keySet();
Review Comment:
keeping same functionality for now, but this is deleting the
DefaultKeySizeConstraint when no default iterators is set, not sure if this is
intentional
##########
test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java:
##########
@@ -517,6 +517,38 @@ public void testNtcChaining() throws AccumuloException,
AccumuloSecurityExceptio
}
}
+ @Test
+ public void testIteratorConflictsWithDefault() throws Exception {
Review Comment:
While looking at NewTableConfiguration, noticed default iterators were not
added to iteratorProps set and only that is checked for iterator conflicts.
This seems like it may be a bug, but wasn't sure. This test demonstrates the
potential bug.
--
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]
