Copilot commented on code in PR #12147:
URL: https://github.com/apache/gravitino/pull/12147#discussion_r3627228371


##########
core/src/main/java/org/apache/gravitino/catalog/CapabilityHelpers.java:
##########
@@ -507,16 +505,28 @@ private static String applyCapabilitiesOnName(
 
   public static String applyCaseSensitiveOnName(
       Capability.Scope scope, String name, Capability capabilities) {
-    return capabilities.caseSensitiveOnName(scope).supported() ? name : 
name.toLowerCase();
+    if (name == null) {
+      return null;
+    }
+    String normalizedName = capabilities.normalizeName(scope, name);
+    if (normalizedName == null) {
+      throw new IllegalArgumentException(
+          String.format(
+              "Capability.normalizeName(%s, %s) must not return null for a 
non-null name",
+              scope, name));
+    }
+    return normalizedName;

Review Comment:
   The IllegalArgumentException message when normalizeName returns null doesn’t 
identify which Capability implementation returned null, making failures harder 
to debug (especially when multiple catalogs/capabilities are in play). 
Including the capability class in the message would make this actionable 
without needing extra logging or a debugger.



-- 
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]

Reply via email to