[GitHub] [iceberg] rdblue commented on a change in pull request #2703: Add more null checks to TableIdentifier

[GitBox <gi...@apache.org>]

rdblue commented on a change in pull request #2703:
URL: https://github.com/apache/iceberg/pull/2703#discussion_r653835143



##########
File path: api/src/main/java/org/apache/iceberg/catalog/TableIdentifier.java
##########
@@ -45,12 +46,14 @@ public static TableIdentifier of(Namespace namespace, String name) {
   }
 
   public static TableIdentifier parse(String identifier) {
+    Preconditions.checkArgument(identifier != null, "Table identifier must be non-null");
     Iterable<String> parts = DOT.split(identifier);
     return TableIdentifier.of(Iterables.toArray(parts, String.class));
   }
 
   private TableIdentifier(Namespace namespace, String name) {
-    Preconditions.checkArgument(name != null && !name.isEmpty(), "Invalid table name %s", name);
+    Preconditions.checkArgument(namespace != null, "Namespace must be non-null");
+    Preconditions.checkArgument(name != null && !name.isEmpty(), "Table name must be non-null/non-empty");

Review comment:
       It's a minor distinction, but I would normally include what the user actually had. If it was null, then it will show up as `Invalid table name (null or empty): null`. Not a big deal the way it is here, though.

##########
File path: api/src/main/java/org/apache/iceberg/catalog/TableIdentifier.java
##########
@@ -36,6 +36,7 @@
   private final String name;
 
   public static TableIdentifier of(String... names) {
+    Preconditions.checkArgument(names != null, "Cannot create table identifier from null array");

Review comment:
       The problem isn't when this is used as a varargs call. It is that this actually creates `of(String[] names)` in the class file. So you can call it directly with a `String` array:
   
   ```java
   String[] levels = null;
   return Namespace.of(levels); // fails
   ```




-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org