ctubbsii commented on a change in pull request #1971:
URL: https://github.com/apache/accumulo/pull/1971#discussion_r646932615
##########
File path:
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
##########
@@ -792,8 +829,14 @@ public void clone(String srcTableName, String
newTableName, CloneConfiguration c
@Override
public void rename(String oldTableName, String newTableName) throws
AccumuloSecurityException,
TableNotFoundException, AccumuloException, TableExistsException {
- checkArgument(newTableName.length() <= MAX_TABLE_NAME_LEN,
- "Table name is longer than " + MAX_TABLE_NAME_LEN + " characters");
+ checkArgument(oldTableName.matches(VALID_TABLENAME_REGEX),
+ "oldTableName must only contain word characters (letters, digits, and
underscores)"
+ + " and cannot exceed 1024 characters");
Review comment:
Old names, before we enforced the length can still be longer 1024; It'd
be important to allow these to be longer, so the rename can actually happen.
However, I will include this for now, and include this fix in a follow-up.
##########
File path:
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
##########
@@ -814,6 +861,9 @@ public void flush(String tableName) throws
AccumuloException, AccumuloSecurityEx
public void flush(String tableName, Text start, Text end, boolean wait)
throws AccumuloException, AccumuloSecurityException,
TableNotFoundException {
checkArgument(tableName != null, "tableName is null");
+ checkArgument(tableName.matches(VALID_TABLENAME_REGEX),
+ "tableName must only contain word characters (letters, digits, and
underscores)"
+ + " and cannot exceed 1024 characters");
Review comment:
There is a lot of repetition here. It seems to me that the
TableValidators class, if it were in the core jar, could validate non-nullness
and the rest, with an appropriate message. However, this is a good start, so I
will do a follow-up change to move the TableValidators into the core jar and
leverage it to shorten these.
##########
File path:
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
##########
@@ -188,6 +189,10 @@ public TableOperationsImpl(ClientContext context) {
@Override
public boolean exists(String tableName) {
checkArgument(tableName != null, "tableName is null");
+ checkArgument(tableName.matches(VALID_TABLENAME_REGEX),
+ "tableName must only contain word characters (letters, digits, and
underscores)"
+ + " and cannot exceed 1024 characters");
Review comment:
Unfortunately, existing names could be longer than 1024, from before we
added the limit. So, we will need to account for that. I will address this in a
follow-up, so this can go ahead and be merged and closed out.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
