[core] Use Locale.ROOT for every case conversion in main sources - #9771
Draft
LuciferYang wants to merge 7 commits into
Draft
[core] Use Locale.ROOT for every case conversion in main sources#9771LuciferYang wants to merge 7 commits into
LuciferYang wants to merge 7 commits into
Conversation
toLowerCaseIfNeed, toLowerCase, and toUpperCase converted with the JVM default locale. Under a Turkish or Azeri default locale, 'I' lowercases to a dotless glyph and 'i' uppercases to a dotted capital, so the case-sensitive=false identifier matching used by CDC table mapping, computed columns, and the Arrow readers silently broke for columns containing 'I'/'i'. Convert with Locale.ROOT, and align the record side of the same CDC flow: CdcRecord.fieldNameLowerCase also lowercased with the default locale, so fixing only the schema side would newly diverge the two halves of the record-schema join under tr/az (previously both sides mangled identically and data still flowed). Assisted-by: GLM-5.3
LuciferYang
marked this pull request as draft
September 13, 2026 03:06
…common/core
Fixing only StringUtils left the same bug in sites that actually throw. Under a
Turkish default locale 'i' uppercases to a dotted capital, so
PartitionMarkDoneAction.valueOf("SUCCESS_FILE") gets "SUCCESS_FİLE" and
IllegalArgumentException, RowKind.fromShortString("+i") stops matching "+I",
JdbcProtocol.valueOf loses SQLITE and MARIADB, and Solaris detection in
OperatingSystem stops recognising its own name. The DLF request signers, option
key lookups, format identifiers and system table names have the same exposure.
Every converted site here is machine-facing: enum names, protocol tokens,
option keys, header names, OS names, format identifiers, hex digits. None
should follow the JVM default locale. BinaryString.toLowerCase/toUpperCase are
left alone: their ASCII path uses Character.toLowerCase and their fallback
already pins Locale.ROOT, so the SQL upper()/lower() transforms over user data
were never locale-dependent.
Co-Authored-By: Claude Code <noreply@anthropic.com>
getIdentifierPrefixOptions lowercased the key to test the prefix and then
sliced the original by the prefix length, which assumes lowercasing preserves
length. It does not: ROOT maps 'İ' to 'i' plus a combining dot. Match
case-insensitively on the original key instead.
RowKind.fromShortString("-d") passes whichever conversion the code uses, since
Turkish differs from ROOT on 'i' and 'I' alone. Only the "+i" case can catch
the bug, so keep that one and say why.
Co-Authored-By: Claude Code <noreply@anthropic.com>
…olowercase-locale
Pins Locale.ROOT on the remaining paimon-flink-cdc conversions, and matches the format prefix against the identifier as written so the option key is sliced at an offset it has.
Covers paimon-filesystems, paimon-hive, paimon-flink-common, paimon-spark, paimon-lumina, the vendored OrcFile copy, and the docs, benchmark and CI tooling. Also makes the Hive clone copy of getIdentifierPrefixOptions length-safe, like the FileFormat original.
…d names The bulk edit matched only paren-bearing calls, so it left every Scala call and, with it, the pre-lowered format-name list that isFormatTable compares against. Also pins Locale.ROOT on the String.format calls that build file names.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
close #9770
String.toLowerCase()andString.toUpperCase()follow the JVM default locale. Under a Turkish or Azeri default,iuppercases to the dottedİandIlowercases to the dotlessı, so any token that is case-folded before being matched or parsed stops matching what it is compared against. This is a real failure, not a theoretical one:CoreOptions.partitionMarkDoneActions()didPartitionMarkDoneAction.valueOf(x.replace('-','_').toUpperCase()), and bothsuccess-fileanddone-partitioncontain ani, so the default configuration threwIllegalArgumentException: No enum constant ...PartitionMarkDoneAction.SUCCESS_FİLE.RowKind.fromShortString("+i")uppercases to+İ, matches no case arm, and threwUnsupportedOperationException.OrcFileresolves the codec withCompressionKind.valueOf(...toUpperCase()), andZLIBcontains anI, soorc.compress = zlibthrew.CachedClientPoolparses the Hive client cache keys withKeyElementType.valueOf(trimmed.toUpperCase()), andUGIcontains anI, so augicache key threwNo enum constant ...KeyElementType.UGİ. The line directly above it already pinnedLocale.ROOTfor theconf:check, so the two halves of one method disagreed.MySqlTypeUtils.getTypeInfouppercases a source type name before switching on it, andINT,BIGINT,DECIMAL,TIMESTAMPand every geometry name contain ani, so CDC type conversion threwDon't support MySQL type 'İNT' yet.andisGeoTypestopped recognizing geometry columns.MultiTablesSinkMode.fromString("DIVIDED")threwUnsupported mode: dıvıded, andLuminaVectorMetric.fromString("cosine")threwNo enum constant ...COSİNE.DistributedLockDialectFactorylostSQLITEandMARIADBthe same way, so JDBC catalog locking failed to resolve its dialect.OperatingSystemlowercasesos.nameand then looks forsolaris, which becomessolarıs, so the OS came backUNKNOWN. The class is duplicated inpaimon-benchmark, which had the same bug.HttpClientlooks for therequest-idheader by lowercased name, so error messages silently lost the request id.StartupModeandFormatenum parsing, mongodb startup modes, the CDC data format identifier, the OSS/OBS/COSN/Jindo credential key maps,ActionFactory, the global index procedures and vector index type names are all exposed the same way.The issue started from identifier matching:
StringUtils.toLowerCaseIfNeeddrives case-insensitive column and table matching, andCdcRecord.fieldNameLowerCaseis the record side of that same join, so the two had to agree or a column silently nulled out.The rule
No case conversion without an explicit locale anywhere under
src/main, Java or Scala. Two greps verify it:git grep -nE '\.to(Lower|Upper)Case\(\)' -- '*/src/main/java/*'returns five lines, all of themBinaryString's own two methods or a call on aBinaryStringreceiver, andgit grep -nE '\.to(Lower|Upper)Case([^(]|$)' -- '*/src/main/scala/*'returns nothing.BinaryStringis already locale-independent: the ASCII path usesCharacter.toLowerCaseand the non-ASCII fallback pinsLocale.ROOT.The Scala half matters more than a count suggests. A first pass matched only calls written with parentheses, which is every Java call and no paren-less Scala one, and that left
SparkSource.FORMAT_NAMESfolding with the default locale whileFormatTableCatalog.isFormatTablehad been moved to ROOT.MOSAICcontains anI, so on atrJVM the list heldmosaıcand the lookup asked formosaic: creating a MOSAIC format table went from working in its uppercase spelling to failing in both.isFormatTablenow compares againstFormatTable.Format.values()withequalsIgnoreCaseinstead of consulting a pre-lowered list, so the answer no longer depends on the locale in effect when a Scala object initialized, which is also what makes it testable.That rule covers the vendored
org.apache.orc.OrcFilecopy underpaimon-format. It is third-party source, but it is source we ship and run, and the failure is reachable from a documented option, so exempting it would have made this "fixed where convenient" rather than a rule. It also covers the docs generator, the cluster benchmark and the CI license checker, which are not shipped but are equally locale-dependent.Four
String.formatcalls are in scope for the same reason and are not case conversions:%drenders through the default locale's digits, and these four build names rather than messages.IcebergPathFactorywrotev%d.metadata.json, which on anar-EG,fa-IR,my-MMorbn-INJVM producedv٥.metadata.json— a file no Iceberg reader resolves and which Paimon's ownv-prefix version scan cannot parse back, whilenewManifestListFiletwo methods up builds its name by concatenation and is unaffected. The other three name local scratch files (LocalKvDb,FileIOChannel,LocalKvStateFactory).String.formatinside exception messages is deliberately left alone: a number rendered in the reader's locale is correct there.The two that are not one word
FileFormat.getIdentifierPrefixOptionsmatched an option key against the lower-cased format identifier and then sliced the key at that prefix's length. Lower-casing can lengthen a string, so the slice offset does not have to exist in the key: with identifierİthe prefix is three characters and the keyİ.is two, which threwStringIndexOutOfBoundsException: begin 3, end 2, length 2. It now matches case-insensitively against the identifier as written, which keeps the two lengths in step, and lower-cases only the key it puts in the result. For an ASCII identifier the two forms are equivalent, which is why the ORC, Parquet and Avro paths are unaffected.HiveTableCloneExtractor.getIdentifierPrefixOptionsis a copy of that method and got the same treatment. Scope worth stating plainly: every identifier Paimon itself passes is lowercase ASCII (avro,orc,parquet,json,csv, and on the Hive clone path onlyavroreaches the method at all, since the others return earlier), so no shipped format can trigger the overrun. What the guard buys is thatregionMatchesreturns false when the region runs past the key, which makes thesubstringprovably in range for any identifier a customFileFormatimplementation might pass. The three tests that use U+0130 document that contract rather than a scenario a user reaches today.One change that is not a machine token
StringUtils.toUpperCase/toLowerCaseare what the CDCupper()andlower()computed columns run on, so this changes data written into the table under atr,azorltdefault locale:lower("ISTANBUL")now persistsistanbulwhere it previously persistedıstanbul. Locale-independence is the behaviour you want there, since otherwise the value depends on which TaskManager ran the job, and it matches whatBinaryStringalready does. It is still a data change rather than a token fix.There is also an upgrade caveat for a table whose schema was inferred under such a locale. The persisted column name is
ıd; after this change both sides of the join produceid, so the old column stops matching and schema evolution can append a second column beside it.TableNameConverterlikewise resolves a different physical name. Being bug-compatible with a locale-dependent schema is not possible while also being correct, so this is a disclosure rather than something the patch works around.Tests
Each test sets a Turkish default locale and restores it.
TurkishLocaleParsingTestcoverspartitionMarkDoneActions()andRowKind.fromShortString("+i"); only+ican discriminate there, because Turkish differs from ROOT oniandIalone.TurkishLocaleTypeNameTestcovers the CDC type path withgetTypeInfo("int").f0andisGeoType("point").MultiTablesSinkModeTestandLuminaVectorMetricTestcover the two remaining enum lookups.TestCachedClientPoolgains theugicache key.FileFormatPrefixOptionsTestandHiveTableCloneExtractorTestcover the prefix slicing on both copies.StringUtilsTestandCdcRecordTestcover the identifier path.FormatTableCatalogTestcovers everyFormatTable.Formatin both spellings under a Turkish default, which is the assertion that fails on the pre-fix code with[MOSAIC].Not covered, deliberately: the four
FileIOcredential key maps build their lookup table in a static initializer, so a test would depend on class-load order rather than on the fix, and theHiveSchema,PaimonMetaHookandPaimonRecordReaderpaths need a live metastore. Those seven files are one-word changes verified by compilation and by the module's existing tests.Verified on JDK 11. The whole reactor builds with checkstyle, spotless, rat and enforcer enabled. Fail-on-base was run for every new test, not reasoned about: reverting the corresponding site produces
No enum constant ...KeyElementType.UGİ,No enum constant ...LuminaVectorMetric.COSİNE,Unsupported mode: dıvıded,expected: "INT" but was: "İNT",isGeoType("point")false,No enum constant ...PartitionMarkDoneAction.SUCCESS_FİLE,Unsupported short string '+i' for row kind., andStringIndexOutOfBoundsException: begin 3, end 2, length 2for the two prefix copies. The ORC, Parquet and Avro format tests that consume the prefix options pass (19 tests).