Copilot commented on code in PR #9257:
URL: https://github.com/apache/gravitino/pull/9257#discussion_r2562871732
##########
core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java:
##########
@@ -370,6 +370,34 @@ public static NameIdentifier
getSchemaIdentifier(NameIdentifier ident)
return NameIdentifier.of(allElems.get(0), allElems.get(1),
allElems.get(2));
}
+ /**
+ * Try to get the table {@link NameIdentifier} from the given {@link
NameIdentifier}.
+ *
+ * @param ident The {@link NameIdentifier} to check.
+ * @return The table {@link NameIdentifier}
+ * @throws IllegalNameIdentifierException If the given {@link
NameIdentifier} does not include
+ * schema name
Review Comment:
The Javadoc comment says "does not include schema name" but for a table
identifier, it should be "does not include table name" since the method
extracts the table identifier from a column identifier (which requires
metalake, catalog, schema, table, and column elements).
```suggestion
* table name
```
##########
core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java:
##########
@@ -370,6 +370,34 @@ public static NameIdentifier
getSchemaIdentifier(NameIdentifier ident)
return NameIdentifier.of(allElems.get(0), allElems.get(1),
allElems.get(2));
}
+ /**
+ * Try to get the table {@link NameIdentifier} from the given {@link
NameIdentifier}.
+ *
+ * @param ident The {@link NameIdentifier} to check.
+ * @return The table {@link NameIdentifier}
+ * @throws IllegalNameIdentifierException If the given {@link
NameIdentifier} does not include
+ * schema name
+ */
+ public static NameIdentifier getTableIdentifier(NameIdentifier ident)
+ throws IllegalNameIdentifierException {
+ NameIdentifier.check(
+ ident.name() != null && !ident.name().isEmpty(),
+ "The name variable in the NameIdentifier must have value.");
+ Namespace.check(
+ ident.namespace() != null && !ident.namespace().isEmpty() &&
ident.namespace().length() > 2,
+ "Table namespace must be non-null and at least 2 level, the input
namespace is %s",
+ ident.namespace());
+
+ List<String> allElems =
+ Stream.concat(Arrays.stream(ident.namespace().levels()),
Stream.of(ident.name()))
+ .collect(Collectors.toList());
+ if (allElems.size() < 4) {
+ throw new IllegalNameIdentifierException(
+ "Cannot create a schema NameIdentifier less than three elements.");
Review Comment:
The error message says "schema NameIdentifier" but this method creates a
table NameIdentifier. The message should be:
```
"Cannot create a table NameIdentifier less than four elements."
```
```suggestion
"Cannot create a table NameIdentifier less than four elements.");
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
