[GitHub] [calcite] zabetak commented on a diff in pull request #3113: [CALCITE-5567] Enable jdk19 in ci

["zabetak (via GitHub)" <gi...@apache.org>]

zabetak commented on code in PR #3113:
URL: https://github.com/apache/calcite/pull/3113#discussion_r1212934318


##########
core/src/main/java/org/apache/calcite/sql/parser/SqlParserUtil.java:
##########
@@ -738,17 +741,7 @@ public static ParsedCollation parseCollation(String in) {
     }
 
     Charset charset = SqlUtil.getCharset(charsetStr);
-    String[] localeParts = localeStr.split("_");
-    Locale locale;
-    if (1 == localeParts.length) {
-      locale = new Locale(localeParts[0]);
-    } else if (2 == localeParts.length) {
-      locale = new Locale(localeParts[0], localeParts[1]);
-    } else if (3 == localeParts.length) {
-      locale = new Locale(localeParts[0], localeParts[1], localeParts[2]);
-    } else {
-      throw RESOURCE.illegalLocaleFormat(localeStr).ex();

Review Comment:
   Before the changes in this PR there was somewhat strict control on what formats are allowed/supported when working with locales and an exception was thrown if the requested locale is now invalid.
   
   Now with the use of `Locale#forLanguageTag` we will never get an error since it appears that we will always fallback to some defaults. 
   
   Instead of using the `Locale#forLanguageTag` we could use the `Locale.Builder.setLanguageTag`, which as far as I understand is not as permissive,  and will thrown an error when the locale is not well formed: https://download.java.net/java/early_access/panama/docs/api/java.base/java/util/Locale.Builder.html#setLanguageTag(java.lang.String)
   
   In the context of this PR I was wondering if the `Locale.Builder.setLanguageTag` is a better alternative to `Locale#forLanguageTag`.
   
   The latest standard can be found here: https://www.iso.org/standard/76584.html 
   It is not free but you may find older versions available online (https://modern-sql.com/standard). The Part 2: Foundations covers the `COLLATE` clause.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org