weiqingy commented on code in PR #2448:
URL: https://github.com/apache/auron/pull/2448#discussion_r3746872698
##########
auron-flink-extension/auron-flink-planner/src/test/java/org/apache/auron/flink/table/runtime/AuronFlinkCalcITCase.java:
##########
@@ -221,6 +222,19 @@ public void testFilterOrComparison() {
assertThat(rows).isEqualTo(Arrays.asList(Row.of(1), Row.of(2)));
}
+ /** UNIX_TIMESTAMP over the per-row {@code ts} string converts to the
native ext function and
+ * yields the epoch seconds. The session timezone is set to Asia/Shanghai
to make the result
+ * deterministic and to exercise timezone propagation into the native
plan. */
+ @Test
+ public void testUnixTimestamp() {
+
tableEnvironment.getConfig().setLocalTimeZone(ZoneId.of("Asia/Shanghai"));
Review Comment:
Good question. I split it because this PR closes the correctness gap:
fixed-offset zones now fall back to Flink instead of reaching the native call
and failing. #2455 tracks the remaining acceleration gap.
Supporting those zones natively is not a big change, but it crosses the
Java/Rust boundary and changes the same gate you're reviewing. `parse_datetime`
and `resolve_offset_secs` currently take `chrono_tz::Tz`, so fixed offsets
require a small generalization. I'd also want that path validated against
`SimpleDateFormat`, as #2409's parser was.
I considered mapping offsets to the `Etc/GMT` names in Java to avoid the
Rust change. POSIX reverses the sign there, though (`GMT-08:00` maps to
`Etc/GMT+8`), so a mistake could silently shift timestamps by twice the offset
rather than merely leave them unaccelerated. Fractional offsets such as
`GMT+05:30` also have no `Etc/GMT` equivalent. #2455 has the details.
The plan-time gate would still remain for unsupported `SystemV/*` ids, so
#2455 would only narrow it.
Since your earlier review was otherwise LGTM and the requested tests are now
included, I thought keeping the Rust work separate would preserve a focused
review scope. Happy to pull #2455 in if you'd rather see the native support
completed in one 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]
