nikolamand-db commented on code in PR #46180:
URL: https://github.com/apache/spark/pull/46180#discussion_r1606736620


##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationFactory.java:
##########
@@ -245,29 +599,26 @@ public static StringSearch getStringSearch(
    * Returns the collation id for the given collation name.
    */
   public static int collationNameToId(String collationName) throws 
SparkException {
-    String normalizedName = collationName.toUpperCase();
-    if (collationNameToIdMap.containsKey(normalizedName)) {
-      return collationNameToIdMap.get(normalizedName);
-    } else {
-      Collation suggestion = Collections.min(List.of(collationTable), 
Comparator.comparingInt(
-        c -> UTF8String.fromString(c.collationName).levenshteinDistance(
-          UTF8String.fromString(normalizedName))));
-
-      Map<String, String> params = new HashMap<>();
-      params.put("collationName", collationName);
-      params.put("proposal", suggestion.collationName);
-
-      throw new SparkException(
-        "COLLATION_INVALID_NAME", 
SparkException.constructMessageParams(params), null);
-    }
+    return Collation.CollationSpec.collationNameToId(collationName);
+  }
+
+  public static Collation fetchCollationUnsafe(int collationId) throws 
SparkException {
+    return Collation.CollationSpec.fetchCollation(collationId);
   }
 
   public static Collation fetchCollation(int collationId) {
-    return collationTable[collationId];
+    try {
+      return fetchCollationUnsafe(collationId);
+    } catch (SparkException e) {
+      return Collation.CollationSpecUTF8Binary.UTF8_BINARY_COLLATION;
+    }

Review Comment:
   Fixed this. We don't throw or swallow errors anymore in `fetchCollation`, 
but rather perform assertions around assumptions which are validated during 
testing. Note that `fetchCollation` is only used internally and assumes that 
valid collation id is passed as parameter, this enables us to remove error 
handling logic; user-facing collation name to id parsing logic does throw error 
and has regular control flow.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to