SEPURI-SAI-KRISHNA commented on issue #12617: URL: https://github.com/apache/seatunnel/issues/12617#issuecomment-5977505752
Thanks for confirming all eleven sites on `dev`. **PR is up: #12618**, opened as a follow-up to #12495 and using the same `Locale.ROOT` treatment. It carries the tr-TR unit tests you asked for, four of them, following the pattern #12495 established of saving `Locale.getDefault()`, setting `tr-TR`, and restoring in a `finally`: - `CalciteSQLEngineTest.testVectorReduceMethodIsLocaleIndependent` - `DoubaoMultimodalModelTest.testBinaryBase64MimeTypeIsLocaleIndependent` - `DoubaoMultimodalModelTest.testFileSuffixModalityDetectionIsLocaleIndependent` - `ModelInvocationCacheKeyTest.keyIsStableAcrossSpellingsUnderAnyDefaultLocale` I checked each one actually guards a regression rather than just passing: reverting the corresponding `Locale.ROOT` back to the bare call fails exactly those four and nothing else. Full module is `Tests run: 1162, Failures: 0`. One measurement worth recording, because it affects how you read the diff. Of the eleven sites, five are reachable defects under tr-TR and the rest are hardening. `BuiltinFunctions:54` in particular is hardening only: I checked all twelve shipped Calcite UDF names and none of them is altered by a Turkish `toUpperCase()`, since none contains the letter i in a position that changes. I kept it in the PR for consistency rather than claiming it was broken. **On `FieldRenameTransform:167/170` and `TableRenameTransform:153/155`**, agreed and they are out of scope here. Those apply a user-requested rename to data rather than matching an internal token, so forcing `Locale.ROOT` would change output for users who are relying on their own locale's casing. That is a behaviour change needing its own discussion, not a silent fix. **On the lint rule**, yes, and I think it is the more valuable half of this. Eleven sites reappeared in `seatunnel-transforms-v2` after #12495 cleaned the `sql` subtree, which is the argument for a guard rather than another sweep. I would rather land it after #12495 and #12618 merge, so the rule goes in against a clean module instead of failing the build on sites those two PRs already fix. Shall I open a separate issue for it now so it is tracked, and raise the PR once both have landed? -- 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]
