AHeise commented on code in PR #29030:
URL: https://github.com/apache/flink/pull/29030#discussion_r3872852523
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/ShowCreateUtil.java:
##########
@@ -184,7 +185,7 @@ public static String buildShowCreateMaterializedTableRow(
.ifPresent(partitionedBy ->
sb.append(formatPartitionedBy(partitionedBy)));
extractFormattedOptions(table.getOptions(), PRINT_INDENT,
additionalSensitiveKeys)
.ifPresent(v -> sb.append("WITH
(\n").append(v).append("\n)\n"));
- sb.append(extractStartMode(table, timeZoneId)).append("\n");
+ sb.append(extractStartMode(table, timeZoneId,
Clock.systemUTC())).append("\n");
Review Comment:
1. Fixed by normalizing the clock to UTC before reading its wall-clock time:
`LocalDateTime.now(clock.withZone(ZoneOffset.UTC))`. Switching to
`Instant.now(clock).minus(amount)` would actually break: `amount` can be a
`Period` for MONTH/QUARTER/YEAR intervals, and `Period` arithmetic on `Instant`
throws `UnsupportedTemporalTypeException` (Instant has no calendar fields) —
`LocalDateTime` is required here to support those units. Added
`extractStartModeFromNowIsUnaffectedByClockZone`, which uses a
`Clock.fixed(..., ZoneId.of("America/New_York"))` and asserts the same output
as the UTC-clock test, proving the mislabeling can't recur regardless of what
zone a future caller's clock carries.
2. `timeZoneId` isn't used in the FROM_NOW branch at all — the
evaluated-timestamp comment is unconditionally rendered in UTC
(`getFormattedLocalDateTime(instant, ZoneOffset.UTC)`), regardless of session
zone. `timeZoneId` only matters for the `FROM_TIMESTAMP ... WITH LOCAL TIME
ZONE` branch below it. So a non-UTC session doesn't change this code path; no
additional coverage needed there.
3. Already fixed in a later push — that fixture's date is now anchored to
match its sibling case (2020-11-12, exactly 1 month before 2020-12-12).
--
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]
