Skip to content

[core] Use Locale.ROOT for every case conversion in main sources - #9771

Open
LuciferYang wants to merge 8 commits into
apache:masterfrom
LuciferYang:fix/stringutils-tolowercase-locale
Open

[core] Use Locale.ROOT for every case conversion in main sources#9771
LuciferYang wants to merge 8 commits into
apache:masterfrom
LuciferYang:fix/stringutils-tolowercase-locale

Conversation

@LuciferYang

@LuciferYang LuciferYang commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Purpose

close #9770

String.toLowerCase() and String.toUpperCase() follow the JVM default locale. Under a Turkish or Azeri default, i uppercases to the dotted İ and I lowercases 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() did PartitionMarkDoneAction.valueOf(x.replace('-','_').toUpperCase()), and both success-file and done-partition contain an i, so the default configuration threw IllegalArgumentException: No enum constant ...PartitionMarkDoneAction.SUCCESS_FİLE.
  • RowKind.fromShortString("+i") uppercases to , matches no case arm, and threw UnsupportedOperationException.
  • OrcFile resolves the codec with CompressionKind.valueOf(...toUpperCase()), and ZLIB contains an I, so orc.compress = zlib threw.
  • CachedClientPool parses the Hive client cache keys with KeyElementType.valueOf(trimmed.toUpperCase()), and UGI contains an I, so a ugi cache key threw No enum constant ...KeyElementType.UGİ. The line directly above it already pinned Locale.ROOT for the conf: check, so the two halves of one method disagreed.
  • MySqlTypeUtils.getTypeInfo uppercases a source type name before switching on it, and INT, BIGINT, DECIMAL, TIMESTAMP and every geometry name contain an i, so CDC type conversion threw Don't support MySQL type 'İNT' yet. and isGeoType stopped recognizing geometry columns.
  • MultiTablesSinkMode.fromString("DIVIDED") threw Unsupported mode: dıvıded, and LuminaVectorMetric.fromString("cosine") threw No enum constant ...COSİNE.
  • DistributedLockDialectFactory lost SQLITE and MARIADB the same way, so JDBC catalog locking failed to resolve its dialect.
  • OperatingSystem lowercases os.name and then looks for solaris, which becomes solarıs, so the OS came back UNKNOWN. The class is duplicated in paimon-benchmark, which had the same bug.
  • HttpClient looks for the request-id header by lowercased name, so error messages silently lost the request id.
  • System table lookup, StartupMode and Format enum 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.toLowerCaseIfNeed drives case-insensitive column and table matching, and CdcRecord.fieldNameLowerCase is 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 them BinaryString's own two methods or a call on a BinaryString receiver, and git grep -nE '\.to(Lower|Upper)Case([^(]|$)' -- '*/src/main/scala/*' returns nothing. BinaryString is already locale-independent: the ASCII path uses Character.toLowerCase and the non-ASCII fallback pins Locale.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_NAMES folding with the default locale while FormatTableCatalog.isFormatTable had been moved to ROOT. MOSAIC contains an I, so on a tr JVM the list held mosaıc and the lookup asked for mosaic: creating a MOSAIC format table went from working in its uppercase spelling to failing in both. isFormatTable now compares against FormatTable.Format.values() with equalsIgnoreCase instead 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.OrcFile copy under paimon-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.format calls are in scope for the same reason and are not case conversions: %d renders through the default locale's digits, and these four build names rather than messages. IcebergPathFactory wrote v%d.metadata.json, which on an ar-EG, fa-IR, my-MM or bn-IN JVM produced v٥.metadata.json — a file no Iceberg reader resolves and which Paimon's own v-prefix version scan cannot parse back, while newManifestListFile two methods up builds its name by concatenation and is unaffected. The other three name local scratch files (LocalKvDb, FileIOChannel, LocalKvStateFactory). String.format inside 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.getIdentifierPrefixOptions matched 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 threw StringIndexOutOfBoundsException: 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.getIdentifierPrefixOptions is 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 only avro reaches the method at all, since the others return earlier), so no shipped format can trigger the overrun. What the guard buys is that regionMatches returns false when the region runs past the key, which makes the substring provably in range for any identifier a custom FileFormat implementation 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 / toLowerCase are what the CDC upper() and lower() computed columns run on, so this changes data written into the table under a tr, az or lt default locale: lower("ISTANBUL") now persists istanbul where 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 what BinaryString already 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 produce id, so the old column stops matching and schema evolution can append a second column beside it. TableNameConverter likewise 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. TurkishLocaleParsingTest covers partitionMarkDoneActions() and RowKind.fromShortString("+i"); only +i can discriminate there, because Turkish differs from ROOT on i and I alone. TurkishLocaleTypeNameTest covers the CDC type path with getTypeInfo("int").f0 and isGeoType("point"). MultiTablesSinkModeTest and LuminaVectorMetricTest cover the two remaining enum lookups. TestCachedClientPool gains the ugi cache key. FileFormatPrefixOptionsTest and HiveTableCloneExtractorTest cover the prefix slicing on both copies. StringUtilsTest and CdcRecordTest cover the identifier path. FormatTableCatalogTest covers every FormatTable.Format in both spellings under a Turkish default, which is the assertion that fails on the pre-fix code with [MOSAIC].

Not covered, deliberately: the four FileIO credential 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 the HiveSchema, PaimonMetaHook and PaimonRecordReader paths 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., and StringIndexOutOfBoundsException: begin 3, end 2, length 2 for the two prefix copies. The ORC, Parquet and Avro format tests that consume the prefix options pass (19 tests).

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
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>
@LuciferYang LuciferYang changed the title [api] Use Locale.ROOT in StringUtils case conversions [core] Use Locale.ROOT for machine-facing case conversions Sep 13, 2026
LuciferYang and others added 2 commits September 13, 2026 14:09
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>
@LuciferYang LuciferYang changed the title [core] Use Locale.ROOT for machine-facing case conversions [core][cdc] Use Locale.ROOT for machine-facing case conversions Sep 13, 2026
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.
@LuciferYang LuciferYang changed the title [core][cdc] Use Locale.ROOT for machine-facing case conversions [core] Use Locale.ROOT for every case conversion in main sources Sep 13, 2026
…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.
@LuciferYang
LuciferYang marked this pull request as ready for review September 13, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Locale-sensitive case conversions break identifier matching, enum parsing and request signing

1 participant