Skip to content

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

Merged
JingsongLi merged 11 commits into
apache:masterfrom
LuciferYang:fix/stringutils-tolowercase-locale
Sep 22, 2026
Merged

JingsongLi merged 11 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. Where upper/lower output is part of a persisted primary or partition key, the same disclosure has a sharper edge: the computed value changes, so a resumed job can address a different key than the rows it wrote earlier. Those tables need a controlled rewrite, or the old expression semantics preserved, rather than a restart.

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

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 12fef4b. Requirement fit: SUPPORTED. Implementation: FINDINGS.

Locale-independent machine tokens and identifier matching have clear operational value. However, changing the shared field conversion without changing CDC's key-list conversion introduces a fresh-job schema regression on Turkish JVMs; this is separate from the existing-table migration caveat disclosed in the PR body. Details and the head/base reproduction are inline.

All 122 focused locale/string/options tests passed with the exact changed classes on JDK 8, and current CI is successful. The additional real CDC schema-construction probe fails on this head and succeeds with the exact-base helper; no live database was used. After fixing the mismatch, the disclosed migration also needs care where upper/lower is part of a persisted primary or partition key: changed computed values can make subsequent DELETEs address a different key. Those jobs need a controlled rewrite/rebuild or preserved legacy expression semantics before resuming, rather than a binary restart alone.

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Normalize CDC key lists with the same locale as fields

buildPaimonSchema uses this helper for field names, but CdcActionCommonUtils.listCaseConvert still maps String::toLowerCase for source/configured primary keys and partition keys. With Locale tr-TR and a case-insensitive catalog, a fresh source column ID with primary key ID now becomes field id and key ıd, and Schema rejects the table. In the non-strict database-sync path, fields id/CITY with configured partition CITY instead become fields [id, city] and partitionKeys=[], silently dropping the requested partitioning.

I compiled the exact helper and CDC schema builder and exercised both paths: this head fails/drops the partition as above; the exact-base helper consistently produces [ıd]/[ıd] and [id, cıty]/[cıty]. JDBC metadata supplies source column/key names without an earlier normalization, so these inputs are reachable. Please convert listCaseConvert with the same explicit locale and add Turkish schema tests for inferred/configured keys and non-strict partition handling. The audit must include method references such as String::toLowerCase, which a search for .toLowerCase() misses.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d741c20.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrections to my numbers above, and two follow-up commits.

The failure counts I posted were from an intermediate state, where only listCaseConvert was reverted and the type-mapping case did not exist yet. Re-measured against the current test:

  • parent-equivalent tree (the conversions reachable from these tests reverted): all five cases fail, 2 assertion failures and 3 exceptions (IllegalStateException from Schema's own allFields.containsAll(primaryKeys) check, IllegalArgumentException from setPrimaryKeys, UnsupportedOperationException from TypeMappingMode.mode);
  • only listCaseConvert reverted: the four schema cases fail, the type-mapping case passes;
  • only TypeMapping.parse reverted: only the type-mapping case fails;
  • head: 5/5 pass.

620f15f came out of reviewing that test. specifiedPrimaryKeyPassesStrictChecking asserted only doesNotThrowAnyException(), which cannot distinguish "the strict path accepted the key" from "it accepted the key and stored a different one"; it now asserts the resulting primary keys. The class is renamed TurkishLocaleCaseFoldingTest, since the fifth case is an option value rather than a schema key.

I also owe you a correction on UpperTransform / LowerTransform. I justified leaving them alone as SQL semantics, which is weaker than the actual reason: they fold a BinaryString, not a java.lang.String. The ASCII paths use Character.toUpperCase(int) / toLowerCase(int) (BinaryString.java:598,632) and the non-ASCII fallbacks are already toString().toUpperCase(Locale.ROOT) / toLowerCase(Locale.ROOT) (:609-611, :643-645). This patch touches neither file, so there is no default-locale dependence there to remove and no computed-column value changes with it. For contrast, Spark's upper under the binary collation reaches UTF8String.toUpperCaseSlow(), which is an unpinned toString().toUpperCase(), but only for non-ASCII input: full-ASCII strings take toUpperCaseAscii(), so upper('istanbul') looks the same either way.

On the completeness question your last paragraph raises: I enumerated the spellings rather than searching for one. Nothing is left in any src/main for no-arg .toLowerCase()/.toUpperCase(), method references on any receiver (zero repo-wide now, tests included), Locale.getDefault(), %S/%T format conversions, Commons/Guava/ICU case helpers, java.text.Collator, Normalizer, Scala's paren-less and .capitalize forms, or valueOf(x.toUpperCase(...))-style enum folding. Six explicit Locale.US conversions remain, in MemorySize, TimeUtils, HadoopFileIO and FlinkFileIO, all on machine tokens; the JDK applies special casing only for the language codes tr, az and lt, so those are byte-identical to ROOT for every input (checked over every defined code point: zero differences for Locale.US, differences under tr-TR). equalsIgnoreCase, CASE_INSENSITIVE_ORDER, regionMatches(true, ...) and Pattern.CASE_INSENSITIVE do not consult the default locale, so they are out of scope.

One judgement call worth naming: paimon-api's StringUtils.toLowerCase is already ROOT-pinned and null-safe, so StringUtils::toLowerCase would have been a literal drop-in for String::toLowerCase. I used inline lambdas instead, because that helper maps null to null and would turn today's NPE on a null key element into a null sitting in a key list. Happy to switch if you prefer the shared helper.

The sweep matched `.toLowerCase()` textually and missed `String::toLowerCase`
method references, so seven sites kept the JVM default locale. One of them is a
correctness bug rather than an inconsistency: CdcActionCommonUtils.buildPaimonSchema
folds field names through toLowerCaseIfNeed (Locale.ROOT) and key lists through
listCaseConvert (default locale), so on a Turkish JVM with a case-insensitive
catalog a source column ID with primary key ID yields field id and key ıd. The
strict path rejects the schema; the non-strict database-sync path drops the
requested partitioning instead.

The other six are the same class: type-mapping option parsing, the Kafka
offset-reset hint, Hive partition key names, Hive predicate-pushdown column
names, and FileIO option keys.

TurkishLocaleSchemaKeyTest covers inferred keys, specified keys under both strict
and non-strict checking, non-strict partition handling, and an upper-case
type-mapping option. All four schema cases fail on the parent commit.
assertThatCode(...).doesNotThrowAnyException() could not tell "the strict path
accepted the key" from "it accepted the key and stored a different one". Assert
the primary keys instead.

Renamed the class: it also covers a --type-mapping option value, which is not a
schema key.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the CDC key-list normalization. Using Locale.ROOT removes locale-dependent casing, including the Turkish-locale failure mode, while preserving the expected key matching behavior. The regression coverage looks good to me.

@JingsongLi

Copy link
Copy Markdown
Contributor

Please resolve conflicts.

…olowercase-locale

# Conflicts:
#	paimon-spark/paimon-spark-common/src/main/scala/org/apache/paimon/spark/catalyst/analysis/ReplacePaimonFunctions.scala
@LuciferYang

Copy link
Copy Markdown
Contributor Author

Please resolve conflicts.

done

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requirement fit: SUPPORTED. The earlier CDC key-list finding is fixed by using the same explicit locale as the field-name conversion, and the targeted Turkish-locale coverage remains in the current diff. I rechecked the latest head after the upstream merge: GitHub now reports it mergeable and clean, and I found no new issue in the locale-conversion changes. The documented migration caveat for existing locale-dependent computed keys still needs attention in rollout notes. I did not rerun the full multi-module suite locally.

@JingsongLi
JingsongLi merged commit 0afddbd into apache:master Sep 22, 2026
26 checks passed
@LuciferYang

Copy link
Copy Markdown
Contributor Author

Thank you @JingsongLi

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

2 participants