SEPURI-SAI-KRISHNA opened a new pull request, #12618:
URL: https://github.com/apache/seatunnel/pull/12618
### Purpose of this pull request
Closes #12617.
Eleven call sites in `seatunnel-transforms-v2` convert case without a
`Locale`, so they follow the worker's default locale. Under `tr-TR` the letter
`i` does not round trip: `"if".toUpperCase()` is `"İF"` and
`"GIF".toLowerCase()` is `"gıf"`.
Same defect class as #12496, fixed for the `sql` subtree by #12495. This
covers the remaining sites, in `calcite`, `nlpmodel` and `validator`, which
@DanielLeens asked for as a separate PR in his #12495 review. Every site
becomes `toUpperCase(Locale.ROOT)` or `toLowerCase(Locale.ROOT)`.
**Five sites are reachable defects today.** Each has a test that fails
without the fix:
| Site | Effect under `tr-TR` |
| --- | --- |
| `calcite/udf/VectorReduceFunction.java:60` | `VECTOR_REDUCE(vec, 2,
'random_projection')` throws `Unknown reduction method`. The upper-casing
exists precisely so a lower-case method resolves. |
| `nlpmodel/embedding/multimodal/ModalityType.java:78` | A URL ending
`.GIF`, `.TIF`, `.AVI`, `.ICO`, `.DIB` or `.SGI` is not detected and falls back
to `TEXT`, so an image is embedded as text. |
| `ModalityType.java:102` | `supportsExtension("TIF")` and `("AVI")` return
`false`. |
| `nlpmodel/embedding/remote/doubao/DoubaoModel.java:388` | The base64
payload goes out as `data:ımage/png;base64,...`. `ModalityGroup.IMAGE` is a
shipped constant, so no unusual input is needed. |
| `nlpmodel/ModelInvocationCacheKey.java:144` | Tokens are lower-cased so
equivalent spellings collapse to one key. Under `tr-TR` the key carries
`provider=openaı`, `modality=ımage` and `format=bınary`, so two equivalent
configurations produce two keys. |
**Six sites are the same pattern but are not reachable today**, and I am
changing them for uniformity rather than because they are broken. I would
rather say so than present eleven fixes as eleven bugs:
- `calcite/udf/BuiltinFunctions.java:54`. None of the 12 shipped UDF names
are altered by a Turkish `toUpperCase()`, because they are already upper case.
Only a third-party `CalciteUdf` whose `functionName()` carries a lower-case `i`
would register under a mangled key.
- `ModalityType.java:60` and
`nlpmodel/embedding/multimodal/PayloadFormat.java:40`. Both feed
`equalsIgnoreCase`, which still matches a dotless `i`, so the conversion is
redundant rather than wrong.
- `validator/DataValidatorTransformConfig.java:168` and `:336`, and
`nlpmodel/llm/remote/AbstractModel.java:123`. Their tokens are `NOT_NULL`,
`RANGE`, `LENGTH`, `REGEX`, `UDF`, `true`, `1`, `yes` and `false`, none of
which carry an `i`. Each is one `UNIQUE` or `IN_LIST` case label away from
breaking silently.
Happy to drop the second group if a reviewer would rather keep the diff to
the five reachable sites, or to delete the two redundant `toLowerCase()` calls
that feed `equalsIgnoreCase` instead of qualifying them. I kept one kind of
change throughout so the diff stays mechanical.
The lint rule @DanielLeens also suggested is deliberately not here.
SeaTunnel has no Checkstyle or forbidden-apis today, so a guard means either a
new build plugin or a `tools/` checker wired into `backend.yml`. That is a CI
decision which should not gate a bug fix, and I will raise it separately.
### Does this PR introduce _any_ user-facing change?
No new option, default, signature or format. Behaviour is unchanged on any
default locale whose case rules match ASCII, which is why the 1158 pre-existing
tests pass untouched. On a worker whose default locale is Turkish or Azeri, the
five sites above stop misbehaving.
### How was this patch tested?
Four tests added to three existing classes, following the pattern already
used in #12495: save `Locale.getDefault()`, set `tr-TR`, restore in a `finally`.
- `CalciteSQLEngineTest.testVectorReduceMethodIsLocaleIndependent`
- `DoubaoMultimodalModelTest.testBinaryBase64MimeTypeIsLocaleIndependent`
-
`DoubaoMultimodalModelTest.testFileSuffixModalityDetectionIsLocaleIndependent`,
which also covers `supportsExtension`
-
`ModelInvocationCacheKeyTest.keyIsStableAcrossSpellingsUnderAnyDefaultLocale`
Full module on JDK 11: `Tests run: 1162, Failures: 0, Errors: 0, Skipped: 0`.
Mutation check. Reverting all eleven sites to the bare calls and re-running
fails exactly these four and nothing else:
```
DoubaoMultimodalModelTest.testBinaryBase64MimeTypeIsLocaleIndependent
MIME type must not depend on the default locale, but was:
data:ımage/png;base64,bW
DoubaoMultimodalModelTest.testFileSuffixModalityDetectionIsLocaleIndependent
expected: <ModalityType.GIF(name=gif, ...)> but was:
<ModalityType.TEXT(name=text, ...)>
ModelInvocationCacheKeyTest.keyIsStableAcrossSpellingsUnderAnyDefaultLocale
expected: <...provider=openaı|...|modality=ımage|format=bınary|...>
but was: <...provider=openai|...|modality=image|format=binary|...>
CalciteSQLEngineTest.testVectorReduceMethodIsLocaleIndependent » Transform
```
That no pre-existing test fails on the revert is the point: this class of
bug is invisible to the current suite.
`spotless:check` passes. No E2E test covers these paths, and none is added,
since the defect is a JVM default-locale property that a container test would
not vary.
--
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]
