SEPURI-SAI-KRISHNA opened a new pull request, #12976: URL: https://github.com/apache/seatunnel/pull/12976
Yes, and thank you for maintaining it: this is a contribution to Apache SeaTunnel. ## Purpose of this pull request Closes #12975. `FORMATDATETIME` and `FROM_UNIXTIME` built their formatter with `DateTimeFormatter.ofPattern(format)`, which captures `Locale.getDefault(Locale.Category.FORMAT)` at construction. Any pattern letter that renders text rather than digits (`MMM`, `MMMM`, `EEE`, `EEEE`, `a`, `G`, `QQQ`) was therefore translated into the language of whichever worker JVM ran the transform. The same job over the same rows wrote different values on different workers, and no error was raised. The clearest evidence that this was unintended is inside the same class. `DAYNAME` and `MONTHNAME` pin English through `DateFormatSymbols.getInstance(Locale.ENGLISH)` in `getMonthsAndWeeks` (`DateTimeFunction.java:327-336`), so the two ways of naming the same day disagreed on the same row. ## Answers to the three questions from AGENTS.md **Which issue, expected behaviour, acceptance criteria.** #12975. Expected: the text these two functions render does not depend on the worker's default locale, and agrees with `DAYNAME` and `MONTHNAME`. Acceptance: a unit test that runs the real SQL engine under non-English defaults and fails if either call site is reverted, plus the existing E2E asserting the English values. **Which modules change, what must stay compatible.** Only `seatunnel-transforms-v2`, two lines in `DateTimeFunction`. No option, API, SPI, schema or checkpoint change. `Locale` was already imported, so there is no import change. Output on an English-locale worker is byte identical, which is why `Locale.ENGLISH` was chosen over `Locale.ROOT`. **Which checks are required.** `spotless:apply` then `spotless:check`, `-DskipTests verify`, and the module test suite. All run, all green; numbers below. The E2E needs Docker, which I could not run here, so I report that honestly rather than claim it. ## The change Two call sites, `DateTimeFunction.java:616` (`formatdatetime`) and `:799` (`fromUnixTime`), now pass `Locale.ENGLISH`. Production diff is +6/-2, of which 4 lines are comments. ### Why `Locale.ENGLISH` and not `Locale.ROOT` The project uses `Locale.ROOT` in 129 places in main source, so it looks like the obvious choice. It is the wrong one here, measured on JDK 17.0.20.1: | pattern | today, en-US | `Locale.ROOT` | `Locale.ENGLISH` | |---|---|---|---| | `EEEE` | Friday | **Fri** | Friday | | `MMMM` | October | **Oct** | October | | `EEE` / `MMM` / `a` / `G` / `QQQ` | Fri / Oct / PM / AD / Q4 | same | same | `Locale.ROOT` changes today's output for 2 of the 7 text patterns; `Locale.ENGLISH` changes 0 of 7. `Locale.ROOT` is right for case conversion, where it avoids the Turkish dotless i, and wrong for date text, where it abbreviates. `Locale.ENGLISH` also matches what this class already does for `DAYNAME` and `MONTHNAME`. ## Measured before and after Compiled from `dev` and called through the real `DateTimeFunction`, input `2026-10-23T14:05:09`. Before: | default locale | `DAYNAME(dt)` | `FORMATDATETIME(dt,'EEEE')` | `FORMATDATETIME(dt,'dd MMM yyyy')` | `FROM_UNIXTIME(ts,'MMM dd','UTC')` | |---|---|---|---|---| | `en-US` | Friday | Friday | 23 Oct 2026 | Oct 23 | | `tr-TR` | Friday | Cuma | 23 Eki 2026 | Eki 23 | | `ja-JP` | Friday | 金曜日 | 23 10月 2026 | 10月 23 | | `de-DE` | Friday | Freitag | 23 Okt. 2026 | Okt. 23 | | `ar-EG` | Friday | الجمعة | 23 أكتوبر 2026 | أكتوبر 23 | After, every locale returns the `en-US` row. Reproduced in a separate JVM with no `Locale.setDefault` call, using only `-Duser.language=tr -Duser.country=TR`. ## Scope kept narrow, and checked rather than assumed - Numeric-only patterns were never affected: `yyyy-MM-dd HH:mm:ss` through the same function gives one distinct result across all five locales. - All 13 `ZetaDateTimeFormat` patterns are numeric-only and produced zero variation across seven locales, including `th-TH-u-ca-buddhist` and `ar-EG-u-nu-arab`. That enum is untouched. - Every existing use of these two functions in the unit tests, the E2E configs and the docs is numeric-only, so nothing pinned the old behaviour and no existing test needed changing. ## Tests `SQLDateTimeFunctionsTest` gains two tests that go through the real `SQLTransform`, run under `ENGLISH`, `tr-TR` and `ja-JP` defaults, and restore the previous locale in a `finally`, following `testAtTimeZone` in the same class. One asserts that `FORMATDATETIME(dt,'EEEE')` equals `DAYNAME(dt)` and `FORMATDATETIME(dt,'MMMM')` equals `MONTHNAME(dt)`, which is the inconsistency this fixes, and both keep a numeric-pattern control. Each test fails only for the call site it covers: | state of the two call sites | result | |---|---| | both pass `Locale.ENGLISH` | `Tests run: 17, Failures: 0, Errors: 0` | | `formatdatetime` reverted | `Failures: 1`, only the FORMATDATETIME test, `expected: <15 Jun 2024> but was: <15 Haz 2024>` | | `fromUnixTime` reverted | `Failures: 1`, only the FROM_UNIXTIME test, `expected: <01 Jan 2023> but was: <01 Oca 2023>` | | both reverted | `Failures: 2` | ## E2E This area already has an E2E, so it is updated rather than left alone. `func_datetime.conf`, which `TestSQLIT` already executes, gains three text bearing columns asserted against `08 Apr 2021`, `Thursday` and `April`. They sit beside the existing `dayname(c3) = Thursday` and `monthname(c4) = April` rules that they must now agree with, which also pins the `Locale.ENGLISH` decision: a later switch to `Locale.ROOT` would produce `Thu` and `Apr` and fail this conf. I could not run the E2E locally, because this machine's user has no access to the Docker socket. The conf was parsed with the same HOCON library SeaTunnel shades to confirm it is valid and that the three rules resolve to the intended values. The `transform-v2-it` lanes in CI will exercise it. ## Validation - `./mvnw spotless:apply -pl seatunnel-transforms-v2` then `spotless:check`: BUILD SUCCESS, no reformatting needed. - `./mvnw -DskipTests verify -pl seatunnel-transforms-v2`: exit 0. - `./mvnw test -pl seatunnel-transforms-v2`: `Tests run: 1491, Failures: 0, Errors: 0, Skipped: 0`. - JDK `openjdk 17.0.20.1 2026-08-18`, Maven wrapper 3.8.4. ## Does this PR introduce any user-facing change? Yes, and it is documented. `docs/en` and `docs/zh` `sql-functions.md` now state that these names are always English, and both `incompatible-changes.md` files carry an entry with Description, Impact and Migration Guide. On an English-locale worker nothing changes; on a non-English worker these names change from the local language to English and become reproducible. ## Interaction with the Zeta SQL keyword lookup Worth stating plainly, because I found it while testing this. Under a Turkish default locale, a lower case `formatdatetime(...)` never reaches this code at all: the engine's keyword lookup upper cases the name without a locale, the dotless i becomes a dotted one, and the job fails with `Unsupported function: formatdatetime`. That is the separate defect in #12496, and the fix is open as #12495. The two are independent and both real. Upper case SQL reaches these functions today on a Turkish worker and silently returns `23 Eki 2026`, which is what this PR fixes and which needs nothing else. Lower case SQL fails earlier, which #12495 fixes. The new unit tests therefore use upper case function names on purpose, so they isolate the formatter locale rather than tripping over the lookup, and the test comment says so. ## Note on #12703 Open PR #12703 replaces the very line this PR changes at `:799`, routing `fromUnixTime` through a new cached `getFormatter`, so whichever lands second needs a trivial rebase. That PR's helper also calls `ofPattern(pattern)` with no locale and additionally parses with it, so I have left a separate note there rather than widen this PR. -- 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]
