Fixes some createtable bugs: by kevinrr888 · Pull Request #5990 · apache/accumulo

kevinrr888 · 2025-11-24T22:17:19Z

Fixes bugs with --copy-config (-cc) and --no-default-iterators (-ndi) options of createtable Shell command:
- Changes to default iterator settings on src table are now carried over to dest table using -cc
- -ndi option did not work. Tables were still created with default iterator props. Now works as expected.
- Results of using -cc and -ndi in conjunction were incorrect (e.g., copy config of src table (which has default iters) to dest table (specify -ndi) would result in a table with default iters)
- Deprecated -ndi in favor of new --no-default-table-props (-ndtp) which functions the same, but better describes the functionality.
Fixes bugs with NewTableConfiguration
- NewTableConfiguration now considers conflicts with default iterators and iterators set (either through attachIterator() or setProperties()). Previously could set an iterator with the same priority or name as the default iterator (VersioningIterator).
- Deprecated NewTableConfiguration.withoutDefaultIterators() in favor of new NewTableConfiguration.withoutDefaults() which functions the same, but better describes the functionality.
New tests to verify expected functionality of createtable command with -cc and -ndi, creating tables with same config as another table via Java API, and NewTableConfiguration with iterator conflicts with default iterators

- Fixes bugs with --copy-config (-cc) and --no-default-iterators (-ndi) options of createtable Shell command: - Changes to default iterator settings on src table are now carried over to dest table using -cc - ndi option did not work. Tables were still created with default iterator props. Now works as expected. - Results of using -cc and -ndi in conjunction were incorrect (e.g., copy config of src table (which has default iters) to dest table (specify -ndi) would result in a table with default iters) - New tests to verify expected functionality of createtable command with -cc and -ndi, creating tables with same config as another table via Java API, and NewTableConfiguration with iterator conflicts with default iterators

kevinrr888 · 2025-11-24T22:22:20Z

+  @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.


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).

Its super confusing, but there may be a way. I made another comment w/ a suggestion on how to do this. Maybe we could improve this API in 4.0 to make it less confusing.

Added suggestion in 797e391. Leaving unresolved for now in case we want to open 4.0 issue for new API

keith-turner

This could be a follow on issue, looking at this PR I think we should impose the following restrictions on the create table shell command options.

the --copy-config option is mutually exclusive with any other option that sets per table props (like --propFile, --prop, --iter`, etc)
the --propFile option is mutually exclusive with any other option that sets per table props.

Thinking this would be good for the same reason mentioned on another comment, that the behavior is non-deterministic. It depends on the order that the shell command applies these options to a shared map and that order could changes. This leads to the possibility that the command could behave differently for the same input and options over time which makes it hard to reason about. Otherwise we should document the order that options are applied and test it, but I do not think that is worthwhile.

Going for the behavior that --copy-config or --propFile exactly specify the tables initial config w/ no surprises or caveats.

keith-turner · 2025-11-25T18:34:01Z

+  @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.


Its super confusing, but there may be a way. I made another comment w/ a suggestion on how to do this. Maybe we could improve this API in 4.0 to make it less confusing.

keith-turner · 2025-11-25T19:06:43Z

-      Set<String> initialProps = IteratorConfigUtil.generateInitialTableProperties(true).keySet();
+      // handles if default props were copied over
+      Set<String> initialProps = IteratorConfigUtil.getInitialTableProperties().keySet();
      initialProps.forEach(initProperties::remove);


This behavior is so confusing that I think we would be better off throwing an exception here. I think the intent of this property is not to add default iterators to a new table, but removing them makes this command non deterministic. Would be good to be able to always predict what the create table command will do for a given set of options. Seems to me throwing an exception would be better than being unpredictable. Also throwing an exception does not silently change the behavior of the command in the background which is really hard to detect for any automation using this command.

Suggested change

initialProps.forEach(initProperties::remove);

if(!Collections.disjoint(initialProps, initProperties.keySet())){

throw //something

}

I agree. This code was broken before these changes, and using -ndi and -cc in conjunction is a bit contradictory and confusing anyways.

keith-turner · 2025-11-25T19:51:43Z

The two options --copy-config and --exclude-parent-properties are so confusing we should probably deprecate both and introduce a new --copy-properties option that has the same behavior as specifying both of those options.

* NewTableConfiguration now considers conflicts with default iterators and those attached via attachIterator. Previously could attach an iterator with the same priority or name as the default iterator (VersioningIterator). * Deprecated NewTableConfiguration.withoutDefaultIterators() in favor of new NewTableConfiguration.withoutDefaults() which function the same, but withoutDefaults() better describes the functionality. * Test changes Co-authored-by: Keith Turner <kturner@apache.org>

… command

kevinrr888 · 2025-11-28T16:24:41Z

I believe all the remaining comments pertain to enforcing mutual exclusion for some of the createtable ops and also #5990 (comment). These should probably be done in a separate issue/PR, so I think this PR is good for re-review

* stricter checks for conflicts between user set properties/iterators and default table properties * expanded on new NewTableConfigurationIT test

PR apache#5990 intruduced stricter checks to NewTableConfiguration to check that properties or iterators set on the NewTableConfiguration do not conflict with default table properties or iterators. These tests were setting properties which did conflict.

PR #5990 introduced stricter checks to NewTableConfiguration to check that properties or iterators set on the NewTableConfiguration do not conflict with default table properties or iterators. These tests were setting properties which did conflict.

kevinrr888 added this to the 2.1.5 milestone Nov 24, 2025

kevinrr888 self-assigned this Nov 24, 2025

kevinrr888 commented Nov 24, 2025

View reviewed changes

ddanielr requested a review from keith-turner November 25, 2025 16:25

keith-turner reviewed Nov 25, 2025

View reviewed changes

kevinrr888 and others added 2 commits November 26, 2025 15:25

Deprecated --no-default-iterators (-ndi) option for Shell createtable…

18914a5

… command

keith-turner reviewed Dec 1, 2025

View reviewed changes

Comment thread core/src/main/java/org/apache/accumulo/core/client/admin/NewTableConfiguration.java Outdated

DomGarguilo reviewed Dec 1, 2025

View reviewed changes

Comment thread test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java Outdated

DomGarguilo reviewed Dec 1, 2025

View reviewed changes

Comment thread core/src/main/java/org/apache/accumulo/core/client/admin/NewTableConfiguration.java Outdated

address review

db8d7f5

keith-turner approved these changes Dec 1, 2025

View reviewed changes

Comment thread test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java Outdated

changes:

10e1079

* stricter checks for conflicts between user set properties/iterators and default table properties * expanded on new NewTableConfigurationIT test

keith-turner reviewed Dec 3, 2025

View reviewed changes

Comment thread test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java Outdated

Comment thread test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java

address review

b343417

keith-turner approved these changes Dec 4, 2025

View reviewed changes

kevinrr888 merged commit 1ab1816 into apache:2.1 Dec 5, 2025
8 checks passed

kevinrr888 deleted the 2.1-feature-5976 branch December 5, 2025 17:09

This was referenced Dec 5, 2025

create table improvements to shell and Java API #6007

Closed

createtable changes #6012

Merged

This was referenced Dec 18, 2025

Fix failing 2.1 tests #6028

Merged

iterator/property conflicts with table defaults #6030

Open

kevinrr888 mentioned this pull request Jan 14, 2026

Prevents iterator conflicts #6040

Merged

-      initialProps.forEach(initProperties::remove);
+      if(!Collections.disjoint(initialProps, initProperties.keySet())){
+        throw //something
+      }

Conversation

kevinrr888 commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevinrr888 Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

keith-turner Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

kevinrr888 Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

keith-turner left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

keith-turner Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

keith-turner Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

kevinrr888 Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

keith-turner commented Nov 25, 2025

Uh oh!

kevinrr888 commented Nov 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kevinrr888 commented Nov 24, 2025 •

edited

Loading

keith-turner left a comment •

edited

Loading