This is an automated email from the ASF dual-hosted git repository.
krathbun pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 53e207f20c createtable changes (#6012)
53e207f20c is described below
commit 53e207f20c7fbd7ae57bb4a446888a861fb6531f
Author: Kevin Rathbun <[email protected]>
AuthorDate: Fri Dec 12 11:08:43 2025 -0500
createtable changes (#6012)
* createtable changes:
- Deprecated --exclude-parent-properties option, in favor of new
--copy-properties option. Previous usage was --exclude-parent-properties with
--copy-config to copy table properties, excluding parent properties. Now done
with single, more clear --copy-properties option.
- Now enforce mutual exclusion with --copy-config, --copy-properties,
--propFile to avoid non-deterministic results
- Now enforce mutual exclusion with --copy-config, --copy-properties,
--propFile and any of the options that set per-table properties to avoid
non-deterministic results
- New ShellCreateTableIT tests
* deprecated --copy-config (-cc)
Deprecated --copy-config due to confusing functionality when used without
--exclude-parent-properties
---
.../shell/commands/CreateTableCommand.java | 87 +++++++++++++-----
.../accumulo/test/shell/ShellCreateTableIT.java | 102 +++++++++++++--------
.../org/apache/accumulo/test/shell/ShellIT.java | 4 +-
3 files changed, 128 insertions(+), 65 deletions(-)
diff --git
a/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java
b/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java
index c730f502a6..88220ad604 100644
---
a/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java
+++
b/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java
@@ -42,7 +42,6 @@ import org.apache.accumulo.core.client.admin.TimeType;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.data.constraints.VisibilityConstraint;
import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
-import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil;
import org.apache.accumulo.shell.Shell;
import org.apache.accumulo.shell.Shell.Command;
import org.apache.accumulo.shell.ShellUtil;
@@ -56,9 +55,12 @@ import org.apache.hadoop.io.Text;
public class CreateTableCommand extends Command {
private Option createTableOptCopySplits;
// copies configuration (property hierarchy: system, namespace, table) into
table properties
+ @Deprecated(since = "2.1.4")
private Option createTableOptCopyConfig;
// copies properties from target table, (omits system and namespace
properties in configuration)
+ @Deprecated(since = "2.1.4")
private Option createTableOptExcludeParentProps;
+ private Option createTableOptCopyProps;
private Option createTableOptSplit;
private Option createTableOptTimeLogical;
private Option createTableOptTimeMillis;
@@ -73,6 +75,7 @@ public class CreateTableCommand extends Command {
private Option createTableOptLocalityProps;
private Option createTableOptIteratorProps;
private Option createTableOptOffline;
+ private OptionGroup copyConfigGroup;
@Override
public int execute(final String fullCommand, final CommandLine cl, final
Shell shellState)
@@ -114,8 +117,21 @@ public class CreateTableCommand extends Command {
+ " only valid when using " + createTableOptCopyConfig.getLongOpt());
}
- if (cl.hasOption(createTableOptCopyConfig.getOpt())) {
- final String oldTable =
cl.getOptionValue(createTableOptCopyConfig.getOpt());
+ for (var copyOpt : copyConfigGroup.getOptions()) {
+ if (cl.hasOption(copyOpt) && (cl.hasOption(createTableNoDefaultIters)
+ || cl.hasOption(createTableNoDefaultTableProps) ||
cl.hasOption(createTableOptInitProp)
+ || cl.hasOption(createTableOptIteratorProps) ||
cl.hasOption(createTableOptFormatter)
+ || cl.hasOption(createTableOptLocalityProps) ||
cl.hasOption(createTableOptEVC))) {
+ throw new IllegalArgumentException(copyOpt.getLongOpt()
+ + " is mutually exclusive with any other option that sets
per-table properties");
+ }
+ }
+
+ if (cl.hasOption(createTableOptCopyConfig) ||
cl.hasOption(createTableOptCopyProps)) {
+ String oldTable = cl.getOptionValue(createTableOptCopyConfig);
+ if (oldTable == null) {
+ oldTable = cl.getOptionValue(createTableOptCopyProps);
+ }
if (!shellState.getAccumuloClient().tableOperations().exists(oldTable)) {
throw new TableNotFoundException(null, oldTable, null);
}
@@ -152,17 +168,31 @@ public class CreateTableCommand extends Command {
}
// 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())) {
+ Map<String,String> srcTableConfig;
+ var hasCopyConfigOpt = cl.hasOption(createTableOptCopyConfig);
+ var hasCopyPropsOpt = cl.hasOption(createTableOptCopyProps);
+ if (hasCopyConfigOpt || hasCopyPropsOpt) {
+ if (hasCopyConfigOpt) {
+ Shell.log.warn("{} is deprecated and will be removed in a future
version. Use {} instead.",
+ createTableOptCopyConfig.getLongOpt(),
createTableOptCopyProps.getLongOpt());
+ String srcTable = cl.getOptionValue(createTableOptCopyConfig);
+ if (cl.hasOption(createTableOptExcludeParentProps)) {
+ Shell.log.warn(
+ "{} is deprecated and will be removed in a future version. Use
{} instead.",
+ createTableOptExcludeParentProps.getLongOpt(),
createTableOptCopyProps.getLongOpt());
+ // copy properties, excludes parent properties in configuration
+ srcTableConfig =
+
shellState.getAccumuloClient().tableOperations().getTableProperties(srcTable);
+ } else {
+ // copy configuration (include parent properties)
+ srcTableConfig =
+
shellState.getAccumuloClient().tableOperations().getConfiguration(srcTable);
+ }
+ } else {
+ String srcTable = cl.getOptionValue(createTableOptCopyProps);
// copy properties, excludes parent properties in configuration
srcTableConfig =
shellState.getAccumuloClient().tableOperations().getTableProperties(srcTable);
- } else {
- // copy configuration (include parent properties)
- srcTableConfig =
-
shellState.getAccumuloClient().tableOperations().getConfiguration(srcTable);
}
srcTableConfig.entrySet().stream()
.filter(entry -> Property.isValidTablePropertyKey(entry.getKey()))
@@ -172,16 +202,12 @@ public class CreateTableCommand extends Command {
ntc = ntc.withoutDefaults();
}
- // if no defaults selected, remove, even if copied from configuration or
properties
- final boolean noDefaultIters =
cl.hasOption(createTableNoDefaultIters.getOpt());
- if (noDefaultIters ||
cl.hasOption(createTableNoDefaultTableProps.getOpt())) {
+ final boolean noDefaultIters = cl.hasOption(createTableNoDefaultIters);
+ if (noDefaultIters || cl.hasOption(createTableNoDefaultTableProps)) {
if (noDefaultIters) {
- Shell.log.warn("{} ({}) option is deprecated and will be removed in a
future version.",
- createTableNoDefaultIters.getLongOpt(),
createTableNoDefaultIters.getOpt());
+ Shell.log.warn("{} is deprecated and will be removed in a future
version. Use {} instead",
+ createTableNoDefaultIters.getLongOpt(),
createTableNoDefaultTableProps.getLongOpt());
}
- // handles if default props were copied over
- Set<String> initialProps =
IteratorConfigUtil.getInitialTableProperties().keySet();
- initialProps.forEach(initProperties::remove);
// prevents default props from being added in create table call
ntc = ntc.withoutDefaults();
}
@@ -333,18 +359,24 @@ public class CreateTableCommand extends Command {
createTableOptCopyConfig =
new Option("cc", "copy-config", true, "table to copy effective
configuration from");
+ createTableOptCopyProps = new Option("cp", "copy-properties", true,
+ "table to copy properties from. Excludes properties from its
parent(s).");
createTableOptExcludeParentProps = new Option(null,
"exclude-parent-properties", false,
- "exclude properties from its parent(s) when copying configuration");
+ String.format(
+ "deprecated; use %s instead. Exclude properties from its parent(s)
when copying configuration.",
+ createTableOptCopyProps.getLongOpt()));
createTableOptCopySplits =
new Option("cs", "copy-splits", true, "table to copy current splits
from");
createTableOptSplit = new Option("sf", "splits-file", true,
"file with a newline-separated list of rows to split the table with");
createTableOptTimeLogical = new Option("tl", "time-logical", false, "use
logical time");
createTableOptTimeMillis = new Option("tm", "time-millis", false, "use
time in milliseconds");
- createTableNoDefaultIters = new Option("ndi", "no-default-iterators",
false,
- "deprecated; use --no-default-table-props (-ndtp) instead. Prevent
creation of the normal default iterator set");
createTableNoDefaultTableProps = new Option("ndtp",
"no-default-table-props", false,
"prevent creation of the default iterator and default key size limit");
+ createTableNoDefaultIters = new Option("ndi", "no-default-iterators",
false,
+ String.format(
+ "deprecated; use %s instead. Prevent creation of the normal
default iterator set",
+ createTableNoDefaultTableProps.getLongOpt()));
createTableOptEVC = new Option("evc", "enable-visibility-constraint",
false,
"prevent users from writing data they cannot read. When enabling this,"
+ " consider disabling bulk import and alter table.");
@@ -354,6 +386,7 @@ public class CreateTableCommand extends Command {
createTableOptInitPropFile =
new Option("pf", "propFile", true, "user-defined initial properties
file");
createTableOptCopyConfig.setArgName("table");
+ createTableOptCopyProps.setArgName("table");
createTableOptCopySplits.setArgName("table");
createTableOptSplit.setArgName("filename");
createTableOptFormatter.setArgName("className");
@@ -383,20 +416,24 @@ public class CreateTableCommand extends Command {
timeGroup.addOption(createTableOptTimeLogical);
timeGroup.addOption(createTableOptTimeMillis);
+ // these table config options are mutually exclusive
+ copyConfigGroup = new OptionGroup();
+ copyConfigGroup.addOption(createTableOptCopyConfig);
+ copyConfigGroup.addOption(createTableOptCopyProps);
+ copyConfigGroup.addOption(createTableOptInitPropFile);
+
base64Opt = new Option("b64", "base64encoded", false, "decode encoded
split points");
o.addOption(base64Opt);
o.addOptionGroup(splitOrCopySplit);
o.addOptionGroup(timeGroup);
- o.addOption(createTableOptSplit);
- o.addOption(createTableOptCopyConfig);
+ o.addOptionGroup(copyConfigGroup);
o.addOption(createTableOptExcludeParentProps);
o.addOption(createTableNoDefaultIters);
o.addOption(createTableNoDefaultTableProps);
o.addOption(createTableOptEVC);
o.addOption(createTableOptFormatter);
o.addOption(createTableOptInitProp);
- o.addOption(createTableOptInitPropFile);
o.addOption(createTableOptLocalityProps);
o.addOption(createTableOptIteratorProps);
o.addOption(createTableOptOffline);
diff --git
a/test/src/main/java/org/apache/accumulo/test/shell/ShellCreateTableIT.java
b/test/src/main/java/org/apache/accumulo/test/shell/ShellCreateTableIT.java
index a723d50dea..8ea3bf70bd 100644
--- a/test/src/main/java/org/apache/accumulo/test/shell/ShellCreateTableIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/shell/ShellCreateTableIT.java
@@ -67,6 +67,8 @@ import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
@Tag(MINI_CLUSTER_ONLY)
@Tag(SUNNY_DAY)
@@ -695,7 +697,7 @@ public class ShellCreateTableIT extends
SharedMiniClusterBase {
@Test
public void copyConfigOptionsTest() throws Exception {
String[] names = getUniqueNames(2);
- String srcNS = "ns1"; // + names[0];
+ String srcNS = "ns1";
String srcTable = srcNS + ".src_table_" + names[1];
String destTable = srcNS + ".dest_table_" + names[1];
@@ -764,13 +766,14 @@ public class ShellCreateTableIT extends
SharedMiniClusterBase {
}
}
- @Test
- public void copyTablePropsOnlyOptionsTest() throws Exception {
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void copyTablePropsOnlyOptionsTest(boolean copyConfig) throws
Exception {
String[] names = getUniqueNames(2);
- String srcNS = "src_ns_" + names[0];
+ String srcNS = "src_ns_" + copyConfig + "_" + names[0];
- String srcTable = srcNS + ".src_table_" + names[1];
- String destTable = srcNS + ".dest_table_" + names[1];
+ String srcTable = srcNS + ".src_table_" + copyConfig + "_" + names[1];
+ String destTable = srcNS + ".dest_table_" + copyConfig + "_" + names[1];
// define constants
final String sysPropName = "table.custom.my_sys_prop";
@@ -786,8 +789,13 @@ public class ShellCreateTableIT extends
SharedMiniClusterBase {
ts.exec("config -s " + nsPropName + "=" + nsPropValue1 + " -ns " + srcNS);
ts.exec("createtable " + srcTable);
- ts.exec("createtable --exclude-parent-properties --copy-config " +
srcTable + " " + destTable,
- true);
+ // (--exclude-parent-properties and --copy-config) should be equivalent to
(--copy-properties)
+ if (copyConfig) {
+ ts.exec("createtable --exclude-parent-properties --copy-config " +
srcTable + " " + destTable,
+ true);
+ } else {
+ ts.exec("createtable --copy-properties " + srcTable + " " + destTable,
true);
+ }
try (AccumuloClient accumuloClient =
Accumulo.newClient().from(getClientProps()).build()) {
Map<String,String> tids = accumuloClient.tableOperations().tableIdMap();
@@ -905,44 +913,64 @@ public class ShellCreateTableIT extends
SharedMiniClusterBase {
}
@Test
- public void testCreateTableNoDefaultIterators1() throws Exception {
- // tests the no default iterators setting
- final String table1 = getUniqueNames(1)[0];
+ public void testCreateTableNoDefaults() throws Exception {
+ // tests the no defaults settings
+ final String[] tableNames = getUniqueNames(2);
+ final String table1 = tableNames[0];
+ final String table2 = tableNames[1];
+ // functionality should be identical
ts.exec("createtable -ndi " + table1, true);
+ ts.exec("createtable -ndtp " + table2, true);
+
// verify no default iterator props
for (IteratorUtil.IteratorScope iterScope :
IteratorUtil.IteratorScope.values()) {
- var res = ts.exec(
- "config -t " + table1 + " -f " + Property.TABLE_ITERATOR_PREFIX +
iterScope.name(), true);
- assertFalse(res.contains(Property.TABLE_ITERATOR_PREFIX +
iterScope.name() + ".vers"));
+ for (String table : tableNames) {
+ var res = ts.exec(
+ "config -t " + table + " -f " + Property.TABLE_ITERATOR_PREFIX +
iterScope.name(),
+ true);
+ assertFalse(res.contains(Property.TABLE_ITERATOR_PREFIX +
iterScope.name() + ".vers"));
+ }
+ }
+ // verify no default table props
+ for (String table : tableNames) {
+ // note filtering by "table.constraint" but checking output for
"table.constraint."
+ // don't want to match command itself
+ var res = ts.exec("config -t " + table + " -f table.constraint", true);
+ assertFalse(res.contains(Property.TABLE_CONSTRAINT_PREFIX.getKey()));
}
}
@Test
- public void testCreateTableNoDefaultIterators2() throws Exception {
- // tests the no default iterators setting
+ public void testCreateTableMutuallyExclusiveCopyOpts() throws Exception {
final String[] tableNames = getUniqueNames(3);
- final String table1 = tableNames[0];
- final String table2 = tableNames[1];
- final String table3 = tableNames[2];
-
- // create table1 with default iterators, create table2 without default
iterators but copying
- // table1 config... Expect no default iterators
- ts.exec("createtable " + table1, true);
- // make sure order doesn't matter
- ts.exec("createtable " + table2 + " -ndi -cc " + table1);
- ts.exec("createtable " + table3 + " -cc " + table1 + " -ndi");
- for (IteratorUtil.IteratorScope iterScope :
IteratorUtil.IteratorScope.values()) {
- var res = ts.exec(
- "config -t " + table1 + " -f " + Property.TABLE_ITERATOR_PREFIX +
iterScope.name(), true);
- // verify default iterator props for table1 but not table2 or table3
- assertTrue(res.contains(Property.TABLE_ITERATOR_PREFIX +
iterScope.name() + ".vers"));
- res = ts.exec(
- "config -t " + table2 + " -f " + Property.TABLE_ITERATOR_PREFIX +
iterScope.name(), true);
- assertFalse(res.contains(Property.TABLE_ITERATOR_PREFIX +
iterScope.name() + ".vers"));
- res = ts.exec(
- "config -t " + table3 + " -f " + Property.TABLE_ITERATOR_PREFIX +
iterScope.name(), true);
- assertFalse(res.contains(Property.TABLE_ITERATOR_PREFIX +
iterScope.name() + ".vers"));
+ final String optArg = "foo";
+ final String src = tableNames[0];
+ final String src2 = tableNames[1];
+ final String dest = tableNames[2];
+
+ ts.exec("createtable " + src, true);
+ ts.exec("createtable " + src2, true);
+
+ var res = ts.exec(String.format("createtable -cc %s -cp %s %s", src, src2,
dest), false);
+ assertTrue(res.contains("AlreadySelectedException"));
+ res = ts.exec(String.format("createtable -cc %s -pf %s %s", src, optArg,
dest), false);
+ assertTrue(res.contains("AlreadySelectedException"));
+ res = ts.exec(String.format("createtable -cp %s -pf %s %s", src, optArg,
dest), false);
+ assertTrue(res.contains("AlreadySelectedException"));
+
+ for (var copyOpt : List.of("-cc", "-cp", "-pf")) {
+ for (var noArgPropOpt : List.of("-ndi", "-ndtp", "-evc")) {
+ res = ts.exec(String.format("createtable %s %s %s %s", noArgPropOpt,
copyOpt, src, dest),
+ false);
+ assertTrue(res.contains("mutually exclusive with"));
+ }
+ for (var propOpt : List.of("-prop", "-i", "-f", "-l")) {
+ res = ts.exec(
+ String.format("createtable %s %s %s %s %s", propOpt, optArg,
copyOpt, src, dest),
+ false);
+ assertTrue(res.contains("mutually exclusive with"));
+ }
}
}
diff --git a/test/src/main/java/org/apache/accumulo/test/shell/ShellIT.java
b/test/src/main/java/org/apache/accumulo/test/shell/ShellIT.java
index bea0a69623..582aaedef8 100644
--- a/test/src/main/java/org/apache/accumulo/test/shell/ShellIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/shell/ShellIT.java
@@ -691,8 +691,7 @@ public class ShellIT extends SharedMiniClusterBase {
writer.println("table.custom.test2=false");
writer.close();
String table = getUniqueNames(1)[0];
- exec("createtable " + table + " --propFile " + file.getAbsolutePath()
- + " -prop table.custom.test3=optional", true);
+ exec("createtable " + table + " --propFile " + file.getAbsolutePath(),
true);
assertTrue(shell.getAccumuloClient().tableOperations().exists(table));
Map<String,String> tableIds =
shell.getAccumuloClient().tableOperations().tableIdMap();
@@ -701,7 +700,6 @@ public class ShellIT extends SharedMiniClusterBase {
getCluster().getServerContext().getTableConfiguration(TableId.of(tableIds.get(table)));
assertEquals("true", tableConf.get("table.custom.test1"));
assertEquals("false", tableConf.get("table.custom.test2"));
- assertEquals("optional", tableConf.get("table.custom.test3"));
}
@Test
