DanielLeens commented on PR #12703:
URL: https://github.com/apache/seatunnel/pull/12703#issuecomment-6391365867
Thanks @SEPURI-SAI-KRISHNA, good catch, and I agree with the analysis. I
confirmed on the current head `2f0c133e8037` that `getFormatter` still builds
with `DateTimeFormatter.ofPattern(pattern)` (`DateTimeFunction.java:65`), and
`unixTimestamp` parses through it (`:872`, `:907`). A text pattern such as `dd
MMM yyyy` therefore depends on the worker's default FORMAT locale, and because
the `DateTimeException` catch turns the failure into NULL, the same config
would silently give different results on different workers. My earlier reviews
missed this, sorry about that.
@chovy-3012, I would treat this as a small follow-up on this PR rather than
something to leave to #12976:
1. Change `getFormatter` to `DateTimeFormatter.ofPattern(pattern,
Locale.ENGLISH)`. `Locale.ENGLISH` rather than `ROOT` is consistent with
`DAYNAME` / `MONTHNAME` in the same class (`:355`) and keeps `EEEE` as
`Saturday`.
2. Add one unit case that does not depend on the machine default, for
example set `Locale.setDefault(Locale.Category.FORMAT, Locale.GERMANY)` (or
`tr-TR`) in a try/finally and assert `UNIX_TIMESTAMP('23 Oct 2026', 'dd MMM
yyyy')` is non-NULL, plus a `FROM_UNIXTIME` text-pattern case. It should fail
without the change.
3. Coordinate with #12976: whichever merges second needs a trivial rebase on
that method, so please mention it in the PR description.
The docs can stay as they are, apart from one line saying text patterns are
interpreted in English. I will re-check once the commit is pushed (this review
is by code reading only, nothing executed locally). Also, the apache-side
`Build` check on `2f0c133e8037` currently shows failure; please re-run only the
failed jobs in the fork once you push, and I will look at the failing lanes
then.
--
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]
