AccumuloTable changes

[kevinrr888]

• Renamed AccumuloTable -> AccumuloNamespace SystemTables
• Added new convenience methods
• Replaced most uses of allTableIds()

The new containsTable(String tableName) and allTableNames() aren't used but added in case they might be helpful in the future. Can remove if desired.

Suggested in: #5487 (comment)

May look like a lot of changes, but is 99% just the rename done through my IDE

[DomGarguilo]

These new contains methods are what I had in mind when I made my original suggestion. LGTM.

[dlmarion]

I'm not sure that this is the best approach. If you look at org.apache.accumulo.core.clientImpl.Namespace I think you will find that AccumuloTable could become Table.java and that AccumuloTable.java is just a specialization of that to represent things in the Accumulo namespace. We have Namespace.java as a first class citizen, but there is no Table.java. AccumuloTable is more of a utility class than anything. I'm not sure what the right answer is here, but I'm not sure this is it.

[ctubbsii]

The rename makes sense for the .containsTable() method, but does not make sense for the ROOT, METADATA, etc. constants. If they were named "ROOT_TABLE", etc., then it might be more clear, but I agree it would be better if there was a more concrete Table object type. I have long had an idea to create one for the public API, with methods on it for scan, rename, delete, etc., and a version internally, for tracking the resolved names and ids together for the lifecycle of an operation. But, those won't necessarily solve this special case of wanting to have hard-coded references to the built-in tables. Those abstractions are better suited for the general use case.

These were previously on the MetadataTable class, but not all the builtin tables are metadata tables, so an attempt was made in #4163 to consolidate them into their own helper utility class.

Maybe a better name is AccumuloNamespaceTables (very clear, but long), or BuiltinTables (shorter, but less clear)?

Perhaps it's better to pass on this change for now. I'm not sure there's a perfect solution, and the current name is adequate, even if it is a little confusing in some cases.

[dlmarion]

I prefer to call them the system namespace / system tables, but that may be due to my experience with relational databases.. Having Table and Namespace objects for the public API might make sense at some point. Having an internal object that represents the objects necessary to maintain the system also makes sense, I think we just need the right naming.

By the way, we also have an object called RootTable which mostly contains constants that could be in Constants.java.

[dlmarion]

I don't have a better name, I'm going to resolve this conversation.

[ctubbsii]

I prefer to call them the system namespace / system tables, but that may be due to my experience with relational databases.. Having Table and Namespace objects for the public API might make sense at some point. Having an internal object that represents the objects necessary to maintain the system also makes sense, I think we just need the right naming.

Would SystemTables make sense as a class name then? I am fine with it. Another way to say it is the accumulo namespace is reserved for system tables, or system tables reside in the accumulo namespace. These are synonymous, but the class name SystemTables makes it clear that it's a utility for system tables, but doesn't misrepresent itself as an abstraction for the namespace itself. I like it.

By the way, we also have an object called RootTable which mostly contains constants that could be in Constants.java.

Yeah, there was an effort to try to localize a lot of narrowly-scoped constants to the code where they were relevant, rather than having a massive list of global constants, which was becoming unwieldy. I would like Constants to go away eventually, or only contain a couple global constants. We should be able to organize the constants that pertain to storage schemas into their respective abstraction layers, so callers don't have to know the schema themselves. I think that one is okay for now, but we may want to revisit once we do have a more general abstraction for tables for use internally.

[dlmarion]

I was thinking one of:

• a class called System, maybe in the org.apache.accumulo.core.schema package
• or a class called SystemSchema

the class would contain the system namespace and system tables and could have methods such as:

boolean isSystemNamespace(NamespaceId)
NamespaceId getSystemNamespace()
boolean isSystemTable(TableId)
Set<TableId> getSystemTables()

[dlmarion]

We could still expose the root, metadata, fate, and scanref tables as constants. Maybe we move Namespace.Accumulo and Namespace.default constants to this new class too.

[ctubbsii]

System is a well-known class name in Java, and I think that something like that may be too generically named. SystemSchema might work, but I'm having trouble thinking of this as a "schema" in any sense of that word I'm familiar with. "SystemTables" seemed to satisfy what you were thinking to me. At its most terse, maybe:

Namespace namespace();
Map<TableId,String> mapping();

In use:

boolean isSystemNamespace = SystemTables.namespace().id().equals(namespaceId);
boolean isSystemTable = SystemTables.mapping().keySet().contains(tableId);

But that's a lot of boilerplate, so having extra methods might help:

String namespaceName();
NamespaceId namespaceId();
Set<TableId> tableIds();
boolean containsTableId(TableId tableId);
boolean containsTableName(String tableName);
Map<String,String> tableIdToNameMap();

Either way, terse or verbose, I think the name "SystemTables" works.

[dlmarion]

Sounds fine to me. I have no objections.

[kevinrr888]

SystemTables also sounds like a good name to me

I made the suggested changes

[dlmarion]

LGTM