LuciferYang commented on code in PR #9771:
URL: https://github.com/apache/paimon/pull/9771#discussion_r4001930988
##########
paimon-api/src/main/java/org/apache/paimon/utils/StringUtils.java:
##########
@@ -668,7 +669,9 @@ public static String quote(String str) {
}
public static String toLowerCaseIfNeed(String str, boolean caseSensitive) {
- return caseSensitive ? str : str.toLowerCase();
+ // Locale.ROOT: identifier matching must not depend on the JVM default
locale
+ // (e.g. Turkish lowercases 'I' to a dotless glyph and breaks column
mapping)
+ return caseSensitive ? str : str.toLowerCase(Locale.ROOT);
Review Comment:
Fixed in d741c20d8.
You are right about the audit gap: I matched `.toLowerCase()` textually, so
every `String::toLowerCase` method reference stayed on the default locale.
`listCaseConvert` is the one with a correctness consequence, and I reproduced
both paths you describe before changing anything.
The same search over main sources turned up six more references in that
form, all machine tokens, so they are converted in the same commit:
`TypeMapping.parse` (an upper-case `--type-mapping` value stops matching a mode
whose name contains an `i`, e.g. `TINYINT1-NOT-BOOL`), the Kafka offset-reset
hint in `KafkaActionUtils`, Hive partition key names in `PaimonMetaHook`,
predicate-pushdown column names in `SearchArgumentToPredicateConverter`, and
option keys in `FileIO`. `grep -rn '::toLowerCase\|::toUpperCase'` over
`src/main` is now empty.
`TurkishLocaleSchemaKeyTest` covers `buildPaimonSchema` under `tr-TR`: a
primary key inferred from the source schema, a specified primary key under both
strict and non-strict checking, and a specified partition key under non-strict
checking, plus the type-mapping case. On the parent commit the four schema
cases fail (3 assertion failures, 1 error); on this head all five pass. Also
ran `TurkishLocaleTypeNameTest`, `CdcRecordTest`, `FileIOTest`,
`StringUtilsTest` and `SearchArgumentToPredicateConverterTest`: green.
On the persisted-value half of your review: this patch does not touch
`UpperTransform` or `LowerTransform`, so no computed column changes value here.
I have added your point to the PR body, since it applies to the disclosed
migration rather than to a code path this patch changes: where `upper`/`lower`
output is part of a primary or partition key, resuming a job after this change
can address a different key, so those tables need a controlled rewrite or
preserved legacy semantics rather than a restart.
--
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]
