raminqaf opened a new pull request, #28146: URL: https://github.com/apache/flink/pull/28146
## What is the purpose of the change [FLINK-39244](https://issues.apache.org/jira/browse/FLINK-39244) added precision 0-9 support to `TO_TIMESTAMP_LTZ`. Follow-up review uncovered several edge cases in the surrounding code: long-overflow at the upper and lower epoch boundaries, missing runtime validation for out-of-range precision, "trailing only" `S` counting in format-based precision inference, an off-by-one nanos overflow in literal serialization, and a plan-time crash when the precision argument is non-literal. This PR fixes all of them and aligns the user-facing documentation accordingly. ## Brief change log - `DateTimeUtils.toTimestampData(double, int)`: compute seconds and sub-second nanos separately to avoid long overflow at the year 0000 and year 9999 boundaries. - `DateTimeUtils.epochToTimestampData`: validate precision is within `[0, 9]` at runtime; throw `TableRuntimeException` otherwise (matches the plan-time `ValidationException` for literal precision). - `DateTimeUtils.precisionFromFormat`: find the longest `S` run outside quoted literal sections (was: trailing `S` count only), clamped to `[3, 9]`. - `ValueLiteralExpression.canRepresentAsLong`: fold the sub-second nanos term into the bound check so instants near `epochSeconds == 9223372036` no longer emit a wrapped negative numeric literal. - `ToTimestampLtzTypeStrategy`: guard `callContext.getArgumentValue(1, Integer.class)` with `isArgumentLiteral(1)`; without it the Calcite-bound context threw `AssertionError: not a literal` for non-literal precision arguments. - **Documentation**: updated `docs/data/sql_functions.yml` and `sql_functions_zh.yml`, Python `to_timestamp_ltz` docstring, and `Expressions.toTimestampLtz` Java Javadocs to reflect the corrected behavior and the literal-vs-non-literal precision/format contract. ## Verifying this change This change added tests and can be verified as follows: - Added IT cases in `TimeFunctionsITCase#toTimestampLtzTestCases` for: - Valid `DOUBLE` epoch column at the `MIN_EPOCH_SECONDS` and `MAX_EPOCH_SECONDS` boundaries round-tripping correctly. - `Double.MAX_VALUE` / `-Double.MAX_VALUE` returning `NULL`. - Non-literal precision: valid values (3, 6) honored at runtime with implicit cast to declared `TIMESTAMP_LTZ(3)`, and out-of-range values (-1, 10) producing a runtime error with the expected message. - Non-literal format columns with both trailing-`S` patterns and non-trailing patterns (`'SSSSSS X'`, `'SSSSSSSSS X'`, `'SSSS\'Z\''`) parsing correctly and truncating to the declared `TIMESTAMP_LTZ(3)`. - Literal non-trailing-`S` patterns yielding the expected `TIMESTAMP_LTZ(6/9/4)` declared types. - Added a boundary case in `ExpressionTest#testTimestampLtzPrecisionAsSerializableString` covering the upper-bound nanos-overflow case (`Instant.ofEpochSecond(9223372036, 900_000_000)` at precision 9) and the negative-base boundary (`Instant.ofEpochSecond(-9223372036L, 0)` at precision 9). ## Does this pull request potentially affect one of the following parts: - **Dependencies** (does it add or upgrade a dependency): **no** - **The public API**, i.e., is any changed class annotated with `@Public(Evolving)`: **yes** — Javadocs on `Expressions.toTimestampLtz` are updated and the user-visible behavior of `TO_TIMESTAMP_LTZ` changes in three ways: 1. Out-of-range precision now throws `TableRuntimeException` instead of producing wrong results or `ArithmeticException`. 2. Output precision for literal patterns with non-trailing `S` runs now reflects the actual fractional-second width. 3. `Instant` literals near the `Long.MAX_VALUE / 10^9` boundary serialize via the string variant instead of a wrapped numeric. - **The serializers**: **no** - **The runtime per-record code paths** (performance sensitive): **yes** — `DateTimeUtils.toTimestampData` and `epochToTimestampData` are on the hot path for `TO_TIMESTAMP_LTZ`. The added `validatePrecision` is a single in-range integer check. - **Anything that affects deployment or recovery**: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: **no** - **The S3 file system connector**: **no** ## Documentation - Does this pull request introduce a new feature? **no** (bugfix) - If yes, how is the feature documented? **not applicable** — but the user-facing behavior documentation (`sql_functions.yml`, `sql_functions_zh.yml`, Python and Java API Javadocs) was corrected as part of this PR. --- **Was generative AI tooling used to co-author this PR?** - [x] Yes (please specify the tool below) Generated-by: Opus 4.7 -- 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]
