k-rus commented on code in PR #4038:
URL: https://github.com/apache/cassandra/pull/4038#discussion_r2281846845
##########
src/java/org/apache/cassandra/schema/TableMetadata.java:
##########
@@ -649,6 +651,17 @@ public void validate()
require((params.transactionalMode == TransactionalMode.off &&
params.transactionalMigrationFrom == TransactionalMigrationFromMode.none) ||
!isCounter(), "Counters are not supported with Accord for table " + this);
}
+ private void validateTableName()
+ {
+ if (!isValidCharsName(name))
+ except("Table name must not be empty or not contain
non-alphanumeric-underscore characters (got \"%s\")", name);
+
+ if (name.length() > TABLE_NAME_LENGTH)
+ except("Table name must not be more than %d characters long (got
%d characters for \"%s\")", TABLE_NAME_LENGTH, name.length(), name);
+
+ assert getTableDirectoryName().length() <= FILENAME_LENGTH :
String.format("Generated directory name for a table of %d characters doesn't
fit the max filename legnth of %s. This unexpectedly wasn't prevented by check
of the table name length, %d, to fit %d characters (got table name \"%s\" and
generated directory name \"%s\"", getTableDirectoryName().length(),
FILENAME_LENGTH, name.length(), TABLE_NAME_LENGTH, name,
getTableDirectoryName());
Review Comment:
This is already covered by the exception on line 659. This assert is to
prevent introducing a bug on line 659. Maybe I am overcautious here and the
assert can be removed. I was hoping that the assert will give more clarity than
confusion. As you see it differently what will be prefered?
##########
src/java/org/apache/cassandra/schema/TableMetadata.java:
##########
@@ -649,6 +651,17 @@ public void validate()
require((params.transactionalMode == TransactionalMode.off &&
params.transactionalMigrationFrom == TransactionalMigrationFromMode.none) ||
!isCounter(), "Counters are not supported with Accord for table " + this);
}
+ private void validateTableName()
+ {
+ if (!isValidCharsName(name))
+ except("Table name must not be empty or not contain
non-alphanumeric-underscore characters (got \"%s\")", name);
+
+ if (name.length() > TABLE_NAME_LENGTH)
+ except("Table name must not be more than %d characters long (got
%d characters for \"%s\")", TABLE_NAME_LENGTH, name.length(), name);
+
+ assert getTableDirectoryName().length() <= FILENAME_LENGTH :
String.format("Generated directory name for a table of %d characters doesn't
fit the max filename legnth of %s. This unexpectedly wasn't prevented by check
of the table name length, %d, to fit %d characters (got table name \"%s\" and
generated directory name \"%s\"", getTableDirectoryName().length(),
FILENAME_LENGTH, name.length(), TABLE_NAME_LENGTH, name,
getTableDirectoryName());
Review Comment:
This is already covered by the exception on line 659. This assert is to
prevent introducing a bug on line 659. Maybe I am overcautious here and the
assert can be removed. I was hoping that the assert will give more clarity than
confusion. As you see it differently what will be preferred?
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
