You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "raminqaf (via GitHub)" <gi...@apache.org> on 2026/05/12 14:51:19 UTC
[PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "raminqaf (via GitHub)" <gi...@apache.org>.
raminqaf commented on PR #28146:
URL: https://github.com/apache/flink/pull/28146#issuecomment-4441687976
@flinkbot run azure
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "featzhang (via GitHub)" <gi...@apache.org>.
featzhang commented on code in PR #28146:
URL: https://github.com/apache/flink/pull/28146#discussion_r3228483304
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/DateTimeUtils.java:
##########
@@ -391,18 +408,29 @@ public static long toEpochValue(Instant instant, int precision) {
}
/**
- * Infers fractional second precision from a format pattern by counting trailing 'S' characters.
- * Returns at least {@link #DEFAULT_PRECISION} (3) and at most 9.
+ * Infers fractional second precision from a format pattern by finding the longest run of 'S'
+ * characters outside quoted literal sections. Returns at least {@link #DEFAULT_PRECISION} (3)
+ * and at most 9.
*/
public static int precisionFromFormat(String format) {
- int sCount = 0;
- for (int i = format.length() - 1; i >= 0; i--) {
- if (format.charAt(i) != 'S') {
- break;
+ int maxRun = 0;
+ int run = 0;
+ boolean inQuotes = false;
Review Comment:
Does the quote-toggling logic handle escaped quotes correctly? In Java's DateTimeFormatter, `''` represents a literal single quote.
For example, pattern `"yyyy-MM-dd HH:mm:ss.SSSS''Z''"` should treat the first `SSSS` as precision 4, with the doubled quotes as escaped literals.
Worth adding a test case:
```java
assertEquals(4, DateTimeUtils.precisionFromFormat("yyyy-MM-dd HH:mm:ss.SSSS''Z''"));
```
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #28146:
URL: https://github.com/apache/flink/pull/28146#issuecomment-4431765972
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "212e72a0a35336e42b78a0f0a9c2e9a8a8810462",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "212e72a0a35336e42b78a0f0a9c2e9a8a8810462",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 212e72a0a35336e42b78a0f0a9c2e9a8a8810462 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "raminqaf (via GitHub)" <gi...@apache.org>.
raminqaf commented on code in PR #28146:
URL: https://github.com/apache/flink/pull/28146#discussion_r3234344395
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/DateTimeUtils.java:
##########
@@ -391,18 +408,29 @@ public static long toEpochValue(Instant instant, int precision) {
}
/**
- * Infers fractional second precision from a format pattern by counting trailing 'S' characters.
- * Returns at least {@link #DEFAULT_PRECISION} (3) and at most 9.
+ * Infers fractional second precision from a format pattern by finding the longest run of 'S'
+ * characters outside quoted literal sections. Returns at least {@link #DEFAULT_PRECISION} (3)
+ * and at most 9.
*/
public static int precisionFromFormat(String format) {
- int sCount = 0;
- for (int i = format.length() - 1; i >= 0; i--) {
- if (format.charAt(i) != 'S') {
- break;
+ int maxRun = 0;
+ int run = 0;
+ boolean inQuotes = false;
Review Comment:
Added more IT Cases that cover the edge-case patterns
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "raminqaf (via GitHub)" <gi...@apache.org>.
raminqaf commented on code in PR #28146:
URL: https://github.com/apache/flink/pull/28146#discussion_r3227441197
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/DateTimeUtils.java:
##########
@@ -380,6 +388,15 @@ private static TimestampData epochToTimestampData(long epoch, int precision) {
return TimestampData.fromInstant(Instant.ofEpochSecond(epochSeconds, nanoAdjustment));
}
+ private static void validatePrecision(int precision) {
+ if (precision < MIN_PRECISION || precision > MAX_PRECISION) {
+ throw new TableRuntimeException(
+ String.format(
+ "Precision for TO_TIMESTAMP_LTZ must be between %d and %d but was %d.",
+ MIN_PRECISION, MAX_PRECISION, precision));
+ }
+ }
Review Comment:
@twalthr I dig deeper here and notice that after introducing the changes FLINK-39244 we made a change in the runtime behavior. So I went back to the old behavior and throw an Exception at this point if the precision is invalid.
Please have a look in the diff:
https://github.com/apache/flink/pull/27757/changes#diff-e29751e95291efb2d7e399b60ee820322a29d24a716b7373a121a5d4d617aefbL382
The old behavior threw a `TableException` for precisions >3
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "twalthr (via GitHub)" <gi...@apache.org>.
twalthr commented on code in PR #28146:
URL: https://github.com/apache/flink/pull/28146#discussion_r3227569437
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/DateTimeUtils.java:
##########
@@ -380,6 +388,15 @@ private static TimestampData epochToTimestampData(long epoch, int precision) {
return TimestampData.fromInstant(Instant.ofEpochSecond(epochSeconds, nanoAdjustment));
}
+ private static void validatePrecision(int precision) {
+ if (precision < MIN_PRECISION || precision > MAX_PRECISION) {
+ throw new TableRuntimeException(
+ String.format(
+ "Precision for TO_TIMESTAMP_LTZ must be between %d and %d but was %d.",
+ MIN_PRECISION, MAX_PRECISION, precision));
+ }
+ }
Review Comment:
Thanks for rechecking this!
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "twalthr (via GitHub)" <gi...@apache.org>.
twalthr merged PR #28146:
URL: https://github.com/apache/flink/pull/28146
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "featzhang (via GitHub)" <gi...@apache.org>.
featzhang commented on code in PR #28146:
URL: https://github.com/apache/flink/pull/28146#discussion_r3228490257
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/expressions/ValueLiteralExpression.java:
##########
@@ -462,11 +462,18 @@ private static String serializeTimestampLtz(Instant instant, int precision) {
/**
* Checks whether an {@link Instant} can be represented as a {@code long} epoch value at the
- * given precision without overflow.
+ * given precision without overflow. Accounts for the sub-second nanos term that {@link
+ * DateTimeUtils#toEpochValue} adds on top of {@code epochSeconds * 10^precision}.
*/
private static boolean canRepresentAsLong(Instant instant, int precision) {
- long factor = (long) Math.pow(10, precision);
- long epochSeconds = instant.getEpochSecond();
- return factor == 0 || Math.abs(epochSeconds) <= Long.MAX_VALUE / factor;
+ final long factor = (long) Math.pow(10, precision);
+ final long nanoDivisor = (long) Math.pow(10, 9 - precision);
+ final long epochSeconds = instant.getEpochSecond();
+ final long nanoPart = instant.getNano() / nanoDivisor;
+ if (epochSeconds > Long.MAX_VALUE / factor || epochSeconds < Long.MIN_VALUE / factor) {
+ return false;
+ }
+ final long base = epochSeconds * factor;
+ return base < 0 || nanoPart <= Long.MAX_VALUE - base;
Review Comment:
The short-circuit for negative base is correct but might benefit from a comment for clarity:
```java
// Negative base + non-negative nanoPart moves toward zero, can't overflow upward
return base < 0 || nanoPart <= Long.MAX_VALUE - base;
```
Since `nanoPart >= 0` always (from `instant.getNano() / divisor`), adding it to a negative base can't exceed `Long.MAX_VALUE`.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "twalthr (via GitHub)" <gi...@apache.org>.
twalthr commented on code in PR #28146:
URL: https://github.com/apache/flink/pull/28146#discussion_r3234393238
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ToTimestampLtzTypeStrategy.java:
##########
@@ -66,19 +70,24 @@ public Optional<DataType> inferType(CallContext callContext) {
}
break;
case 2:
- LogicalType secondType = argumentTypes.get(1).getLogicalType();
+ LogicalType secondType =
+ argumentTypes.get(PRECISION_OR_FORMAT_ARG).getLogicalType();
LogicalTypeRoot secondTypeRoot = secondType.getTypeRoot();
if (firstType.is(LogicalTypeFamily.NUMERIC)) {
if (secondTypeRoot != LogicalTypeRoot.INTEGER) {
throw new ValidationException(
"Unsupported argument type. "
+ "TO_TIMESTAMP_LTZ(<NUMERIC>, <INTEGER>) requires the second argument to be <INTEGER>.");
}
- Optional<Integer> precisionOpt = callContext.getArgumentValue(1, Integer.class);
- if (precisionOpt.isPresent()) {
- int precision = precisionOpt.get();
- validatePrecision(precision);
- outputPrecision = Math.max(precision, DEFAULT_PRECISION);
+ if (callContext.isArgumentLiteral(PRECISION_OR_FORMAT_ARG)) {
Review Comment:
The warning is a nice idea, but we should not log on these "little things" given the size of logs a Flink JM/TM can produce. Type inference might be executed multiple times during planning, it would spam the logs. Also the logs end up in SQL gateway, far away from enduser. Thus, remain unnoticed. I you are not strong on this, I would revert this part.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "raminqaf (via GitHub)" <gi...@apache.org>.
raminqaf commented on PR #28146:
URL: https://github.com/apache/flink/pull/28146#issuecomment-4443697852
@flinkbot run azure
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "raminqaf (via GitHub)" <gi...@apache.org>.
raminqaf commented on code in PR #28146:
URL: https://github.com/apache/flink/pull/28146#discussion_r3234339581
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ToTimestampLtzTypeStrategy.java:
##########
@@ -66,19 +70,24 @@ public Optional<DataType> inferType(CallContext callContext) {
}
break;
case 2:
- LogicalType secondType = argumentTypes.get(1).getLogicalType();
+ LogicalType secondType =
+ argumentTypes.get(PRECISION_OR_FORMAT_ARG).getLogicalType();
LogicalTypeRoot secondTypeRoot = secondType.getTypeRoot();
if (firstType.is(LogicalTypeFamily.NUMERIC)) {
if (secondTypeRoot != LogicalTypeRoot.INTEGER) {
throw new ValidationException(
"Unsupported argument type. "
+ "TO_TIMESTAMP_LTZ(<NUMERIC>, <INTEGER>) requires the second argument to be <INTEGER>.");
}
- Optional<Integer> precisionOpt = callContext.getArgumentValue(1, Integer.class);
- if (precisionOpt.isPresent()) {
- int precision = precisionOpt.get();
- validatePrecision(precision);
- outputPrecision = Math.max(precision, DEFAULT_PRECISION);
+ if (callContext.isArgumentLiteral(PRECISION_OR_FORMAT_ARG)) {
Review Comment:
Thanks for pointing this out. I have added the log.warning to places where we fallback to the default precision during planing
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/expressions/ValueLiteralExpression.java:
##########
@@ -462,11 +462,18 @@ private static String serializeTimestampLtz(Instant instant, int precision) {
/**
* Checks whether an {@link Instant} can be represented as a {@code long} epoch value at the
- * given precision without overflow.
+ * given precision without overflow. Accounts for the sub-second nanos term that {@link
+ * DateTimeUtils#toEpochValue} adds on top of {@code epochSeconds * 10^precision}.
*/
private static boolean canRepresentAsLong(Instant instant, int precision) {
- long factor = (long) Math.pow(10, precision);
- long epochSeconds = instant.getEpochSecond();
- return factor == 0 || Math.abs(epochSeconds) <= Long.MAX_VALUE / factor;
+ final long factor = (long) Math.pow(10, precision);
+ final long nanoDivisor = (long) Math.pow(10, 9 - precision);
+ final long epochSeconds = instant.getEpochSecond();
+ final long nanoPart = instant.getNano() / nanoDivisor;
+ if (epochSeconds > Long.MAX_VALUE / factor || epochSeconds < Long.MIN_VALUE / factor) {
+ return false;
+ }
+ final long base = epochSeconds * factor;
+ return base < 0 || nanoPart <= Long.MAX_VALUE - base;
Review Comment:
Added
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "raminqaf (via GitHub)" <gi...@apache.org>.
raminqaf commented on code in PR #28146:
URL: https://github.com/apache/flink/pull/28146#discussion_r3227498154
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/DateTimeUtils.java:
##########
@@ -391,18 +408,29 @@ public static long toEpochValue(Instant instant, int precision) {
}
/**
- * Infers fractional second precision from a format pattern by counting trailing 'S' characters.
- * Returns at least {@link #DEFAULT_PRECISION} (3) and at most 9.
+ * Infers fractional second precision from a format pattern by finding the longest run of 'S'
+ * characters outside quoted literal sections. Returns at least {@link #DEFAULT_PRECISION} (3)
+ * and at most 9.
*/
public static int precisionFromFormat(String format) {
- int sCount = 0;
- for (int i = format.length() - 1; i >= 0; i--) {
- if (format.charAt(i) != 'S') {
- break;
+ int maxRun = 0;
+ int run = 0;
+ boolean inQuotes = false;
+ for (int i = 0; i < format.length(); i++) {
+ char c = format.charAt(i);
+ if (c == '\'') {
+ inQuotes = !inQuotes;
+ maxRun = Math.max(maxRun, run);
+ run = 0;
+ } else if (!inQuotes && c == 'S') {
+ run++;
+ } else {
+ maxRun = Math.max(maxRun, run);
+ run = 0;
Review Comment:
harden the code for edge cases like:
- `yyyy-MM-dd'T'HH:mm:ss.SSSSSS`
- `yyyy-MM-dd'T'HH:mm:ss.SSSSSSSSSX`
- `yyyy-MM-dd'T'HH:mm:ss.SSSSSSSSS'Z'`
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39666][table] Fix edge-case bugs in TO_TIMESTAMP_LTZ precision handling [flink]
Posted by "featzhang (via GitHub)" <gi...@apache.org>.
featzhang commented on code in PR #28146:
URL: https://github.com/apache/flink/pull/28146#discussion_r3228492939
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ToTimestampLtzTypeStrategy.java:
##########
@@ -66,19 +70,24 @@ public Optional<DataType> inferType(CallContext callContext) {
}
break;
case 2:
- LogicalType secondType = argumentTypes.get(1).getLogicalType();
+ LogicalType secondType =
+ argumentTypes.get(PRECISION_OR_FORMAT_ARG).getLogicalType();
LogicalTypeRoot secondTypeRoot = secondType.getTypeRoot();
if (firstType.is(LogicalTypeFamily.NUMERIC)) {
if (secondTypeRoot != LogicalTypeRoot.INTEGER) {
throw new ValidationException(
"Unsupported argument type. "
+ "TO_TIMESTAMP_LTZ(<NUMERIC>, <INTEGER>) requires the second argument to be <INTEGER>.");
}
- Optional<Integer> precisionOpt = callContext.getArgumentValue(1, Integer.class);
- if (precisionOpt.isPresent()) {
- int precision = precisionOpt.get();
- validatePrecision(precision);
- outputPrecision = Math.max(precision, DEFAULT_PRECISION);
+ if (callContext.isArgumentLiteral(PRECISION_OR_FORMAT_ARG)) {
Review Comment:
The guard prevents the plan-time crash correctly. Nice fix.
One thought: could we add a plan-time warning when precision is non-literal? Users might not realize they're losing sub-millisecond precision due to the default TIMESTAMP_LTZ(3) output type.
Something like:
```java
if (!callContext.isArgumentLiteral(PRECISION_OR_FORMAT_ARG)) {
// LOG.warn("TO_TIMESTAMP_LTZ: non-literal precision defaults to 3...");
return DEFAULT_PRECISION;
}
```
The documentation you added is clear, but a warning might help users catch unintended precision loss at development time. Not critical though.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org