You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@auron.apache.org by "weiqingy (via GitHub)" <gi...@apache.org> on 2026/08/05 04:58:44 UTC
[PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
weiqingy opened a new pull request, #2448:
URL: https://github.com/apache/auron/pull/2448
# Which issue does this PR close?
Closes #1863.
This is the Flink Java side of `UNIX_TIMESTAMP`. The native function it calls merged in #2409.
The 0-argument form is still not supported and is not part of this PR. It is a different function rather than a missing branch: Flink binds the niladic form to `DateTimeUtils.unixTimestamp()`, which reads the clock per record and parses nothing, so it needs its own design pass. The converter rejects it explicitly and falls back to Flink's engine. Happy to file a separate issue for it if you would rather keep that tracked.
# Rationale for this change
This completes native support for Flink's `UNIX_TIMESTAMP` by wiring the Flink Calc converter to emit the native function added in #2409. Without it the native function is unreachable, and any Calc containing `UNIX_TIMESTAMP` falls back to Flink's engine for the whole Calc.
# What changes are included in this PR?
The converter recognizes `UNIX_TIMESTAMP` and lowers the 1-argument and 2-argument forms to the native `Flink_UnixTimestamp` node.
`UNIX_TIMESTAMP` resolves to `SqlKind.OTHER_FUNCTION`, so it is matched by reference identity on the operator before the supported-kinds switch, the same way `TRY_CAST` is handled.
A format scanner translates the supported subset of Java date-format letters (`yyyy MM dd HH mm ss` and literals) to the native format. Anything outside that subset falls back: other pattern letters, unsupported run-lengths, a non-literal format argument, the 0-argument form, and a numeric field adjacent to another numeric field where the run-length would not survive translation (for example `yyyyMd`). Falling back keeps results correct rather than risking a silent divergence.
The session time zone is resolved at plan time and passed into the node. This required completing the config threading in the standalone-Calc path, which passed a persisted config that does not carry `table.local-time-zone`. That gap had no effect until now: `UNIX_TIMESTAMP` is the first time-zone-sensitive expression the converter supports, and the earlier ones (arithmetic, comparison, logical, cast) never read the session zone. The effective node config is threaded through instead, so the configured zone reaches the native evaluation.
# Are there any user-facing changes?
Yes. `UNIX_TIMESTAMP(string)` and `UNIX_TIMESTAMP(string, format)` now execute on the native engine when the format is a supported literal pattern. Unsupported patterns and the 0-argument form continue to run on Flink's engine, with the same results as before.
# How was this patch tested?
Unit tests for the scanner (accept/reject, quote escaping, the adjacency rule), the converter (node shape, format translation, time-zone propagation), and the operator-identity invariant. Fallback tests assert that unsupported inputs actually fall back rather than silently producing a native plan.
An end-to-end ITCase runs `UNIX_TIMESTAMP(ts)` with a non-UTC session zone and confirms the native result matches the expected epoch values. The executed native plan shows the function and the resolved zone, confirming the query runs natively rather than falling back.
160 tests pass in `auron-flink-planner` on the rebased branch, with spotless clean and 0 checkstyle violations.
# Was this patch authored or co-authored using generative AI tooling?
- [x] Yes
- [ ] No
Generated-by: Claude Code (Claude Opus 5)
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #2448:
URL: https://github.com/apache/auron/pull/2448#discussion_r3718190648
##########
auron-flink-extension/auron-flink-planner/src/main/java/org/apache/auron/flink/table/planner/converter/RexLiteralConverter.java:
##########
@@ -112,6 +112,37 @@ private static boolean isSupportedType(SqlTypeName typeName) {
return SUPPORTED_TYPES.contains(typeName);
}
+ /**
+ * Builds a literal expression node from a plain {@link String} that is not backed by a
+ * {@link RexNode}. Used for plan-time constants (such as a session timezone read from
+ * configuration) that must travel to the native side as a string argument. The value is
+ * serialized as a single-element {@code Utf8} Arrow record batch in IPC stream format, matching
+ * the encoding {@link #convert} produces for CHAR/VARCHAR {@link RexLiteral}s.
+ *
+ * @param value the constant string to encode
+ * @return a {@link PhysicalExprNode} carrying the value as a native literal
+ */
+ public static PhysicalExprNode stringLiteral(String value) {
+ RowType rowType = RowType.of(new VarCharType(VarCharType.MAX_LENGTH));
+ try (BufferAllocator allocator =
+ FlinkArrowUtils.getRootAllocator().newChildAllocator("literal", 0, Long.MAX_VALUE);
+ VectorSchemaRoot root = VectorSchemaRoot.create(FlinkArrowUtils.toArrowSchema(rowType), allocator)) {
+
+ GenericRowData rowData = new GenericRowData(1);
+ rowData.setField(0, StringData.fromString(value));
+
Review Comment:
Added a `requireNonNull` in `cccb1acb`, though for a different reason than the one given here.
Null cannot reach this method today. It has two call sites, both in `RexCallConverter.buildUnixTimestamp`: one passes `chronoFormat`, which comes out of `translate(...).orElseThrow(...)` and so is non-null by construction, and the other passes `zone.getId()`, which `ZoneId` never returns null from.
On the diagnosis being opaque: conversion failures are already caught. `FlinkNodeConverterFactory.convertRexNode` wraps the `convert` call in `catch (Exception e)` and logs `RexNode conversion failed for {}` with the full stack trace, then returns empty so the Calc falls back to Flink. So an NPE here would surface as a logged stack trace plus a query that still returns correct results on Flink's engine, not an opaque failure.
What I did take from this: the method is public API introduced by this PR, and a null value would mean the caller failed to resolve a plan-time constant. Encoding that as a NULL literal would ship a silently wrong argument to the native side, so rejecting it at the boundary is the right contract to state. Guard plus javadoc plus `testStringLiteralRejectsNull`.
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #2448:
URL: https://github.com/apache/auron/pull/2448#discussion_r3737899201
##########
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:
Thanks for the review.
The converter resolves the session zone at plan time and hands the id to the native function, which only knows IANA zone names. A fixed offset isn't one, so it errors during execution:
```
Flink_UnixTimestamp: invalid timezone GMT-08:00
```
By that point the plan is already native and the Calc operator has no runtime fallback, so the task dies. It also doesn't need an explicit `SET`. The default resolves to the JVM's default zone, so a TaskManager running with `TZ=GMT-08:00` hits it with no config at all.
Fixed by checking the zone at plan time. If native can't resolve it, the Calc falls back to Flink and returns correct results, the same way the converter already handles formats it can't translate. That covers the `GMT±HH:MM` forms and the legacy `SystemV/*` ids.
Added the tests for: `GMT-08:00` and `UTC` IT cases, plus unit tests on the gate.
Fixed-offset sessions fall back for now rather than running natively. I'll follow up with native support for them so they stay on the native path.
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "Tartarus0zm (via GitHub)" <gi...@apache.org>.
Tartarus0zm commented on code in PR #2448:
URL: https://github.com/apache/auron/pull/2448#discussion_r3733069765
##########
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:
The config `table.local-time-zone` supports multiple formats, such as `GMT-08:00`, I'd suggest adding more test cases for 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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "Tartarus0zm (via GitHub)" <gi...@apache.org>.
Tartarus0zm commented on code in PR #2448:
URL: https://github.com/apache/auron/pull/2448#discussion_r3746761227
##########
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:
@weiqingy What's the rationale for not fixing this issue in this PR and instead opening a separate issue to follow up on it?
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on PR #2448:
URL: https://github.com/apache/auron/pull/2448#issuecomment-5187853780
Hi @Tartarus0zm, could you please help review this PR when you get a chance? Thanks!
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #2448:
URL: https://github.com/apache/auron/pull/2448#discussion_r3737899201
##########
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:
Thanks for the review.
The converter resolves the session zone at plan time and hands the id to the native function, which only knows IANA zone names. A fixed offset isn't one, so it errors during execution:
```
Flink_UnixTimestamp: invalid timezone GMT-08:00
```
By that point the plan is already native and the Calc operator has no runtime fallback, so the task dies. It also doesn't need an explicit `SET`. The default resolves to the JVM's default zone, so a TaskManager running with `TZ=GMT-08:00` hits it with no config at all.
Fixed by checking the zone at plan time. If native can't resolve it, the Calc falls back to Flink and returns correct results, the same way the converter already handles formats it can't translate. That covers the `GMT±HH:MM` forms and the legacy `SystemV/*` ids.
Added the tests for: `GMT-08:00` and `UTC` IT cases, plus unit tests on the gate.
Fixed-offset sessions fall back for now rather than running natively. Filed #2455 to follow up with native support for them so they stay on the native path.
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2448:
URL: https://github.com/apache/auron/pull/2448#discussion_r3718079582
##########
auron-flink-extension/auron-flink-planner/src/main/java/org/apache/auron/flink/table/planner/converter/RexLiteralConverter.java:
##########
@@ -112,6 +112,37 @@ private static boolean isSupportedType(SqlTypeName typeName) {
return SUPPORTED_TYPES.contains(typeName);
}
+ /**
+ * Builds a literal expression node from a plain {@link String} that is not backed by a
+ * {@link RexNode}. Used for plan-time constants (such as a session timezone read from
+ * configuration) that must travel to the native side as a string argument. The value is
+ * serialized as a single-element {@code Utf8} Arrow record batch in IPC stream format, matching
+ * the encoding {@link #convert} produces for CHAR/VARCHAR {@link RexLiteral}s.
+ *
+ * @param value the constant string to encode
+ * @return a {@link PhysicalExprNode} carrying the value as a native literal
+ */
+ public static PhysicalExprNode stringLiteral(String value) {
+ RowType rowType = RowType.of(new VarCharType(VarCharType.MAX_LENGTH));
+ try (BufferAllocator allocator =
+ FlinkArrowUtils.getRootAllocator().newChildAllocator("literal", 0, Long.MAX_VALUE);
+ VectorSchemaRoot root = VectorSchemaRoot.create(FlinkArrowUtils.toArrowSchema(rowType), allocator)) {
+
+ GenericRowData rowData = new GenericRowData(1);
+ rowData.setField(0, StringData.fromString(value));
+
Review Comment:
stringLiteral(String) doesn’t define behavior for a null input. Passing null currently throws a NullPointerException from StringData.fromString(value), which is hard to diagnose and can surface as an opaque planner failure. Add an explicit null check with a clear exception (or encode a NULL literal if that’s intended).
##########
auron-flink-extension/auron-flink-planner/src/main/java/org/apache/auron/flink/table/planner/converter/FlinkDateTimeFormatConverter.java:
##########
@@ -0,0 +1,227 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.auron.flink.table.planner.converter;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * Translates a {@code java.text.SimpleDateFormat} pattern into the native parser's
+ * {@code strftime}-style format string, or reports that the pattern cannot be translated.
+ *
+ * <p>The native {@code Flink_UnixTimestamp} function replicates {@code SimpleDateFormat} lenient
+ * parsing only for a fixed set of numeric fields. This converter is the plan-time gate: it accepts
+ * a pattern only when every token maps to a field the native parser handles identically, and
+ * returns {@link Optional#empty()} otherwise so the whole {@code Calc} falls back to Flink's engine.
+ *
+ * <p>Accepted fields and their native specifiers:
+ * <ul>
+ * <li>{@code yyy} / {@code yyyy} → {@code %Y} (year)
+ * <li>{@code M} / {@code MM} → {@code %m} (month)
+ * <li>{@code d} / {@code dd} → {@code %d} (day of month)
+ * <li>{@code H} / {@code HH} → {@code %H} (hour of day)
+ * <li>{@code m} / {@code mm} → {@code %M} (minute)
+ * <li>{@code s} / {@code ss} → {@code %S} (second)
+ * </ul>
+ *
+ * <p>Non-alphabetic characters are literals (a literal {@code %} is emitted as {@code %%}).
+ * {@code SimpleDateFormat} single-quote escaping applies, including the doubled {@code ''} that
+ * denotes a literal quote. Every other ASCII letter and every unlisted run length forces a fall
+ * back, because Java reserves all letters and the omitted forms either depend on a locale or on the
+ * clock at parse time.
+ *
+ * <p>Adjacency rule: run length is erased by the translation ({@code M} and {@code MM} both become
+ * {@code %m}), yet the native parser reads each numeric field at a canonical width (year 4; month,
+ * day, hour, minute, second 2) while Java's lenient scan window equals the run length. When two
+ * numeric fields are adjacent with no literal separator, those two widths must agree, so the left
+ * field's run length must equal its canonical width; otherwise the pattern falls back to avoid a
+ * silent divergence (e.g. {@code yyyyMd} on {@code 20201010} yields month 1 in Java but month 10
+ * natively).
+ */
+public final class FlinkDateTimeFormatConverter {
+
+ private FlinkDateTimeFormatConverter() {
+ // utility class
+ }
+
+ /**
+ * Translates the given Java {@code SimpleDateFormat} pattern to the native {@code strftime}-style
+ * format string.
+ *
+ * @param javaPattern the Java date-time format pattern
+ * @return the translated native format string, or {@link Optional#empty()} if any part of the
+ * pattern is outside the natively supported surface
+ */
+ public static Optional<String> translate(String javaPattern) {
+ List<Token> tokens = scan(javaPattern);
+ if (tokens == null) {
Review Comment:
translate(String) will throw a NullPointerException for a null javaPattern (scan() calls pattern.length()). Since this is a public utility used as a plan-time gate, it’s safer to treat null as “not translatable” and return Optional.empty() (or explicitly reject null with a clear exception).
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #2448:
URL: https://github.com/apache/auron/pull/2448#discussion_r3718191290
##########
auron-flink-extension/auron-flink-planner/src/main/java/org/apache/auron/flink/table/planner/converter/FlinkDateTimeFormatConverter.java:
##########
@@ -0,0 +1,227 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.auron.flink.table.planner.converter;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * Translates a {@code java.text.SimpleDateFormat} pattern into the native parser's
+ * {@code strftime}-style format string, or reports that the pattern cannot be translated.
+ *
+ * <p>The native {@code Flink_UnixTimestamp} function replicates {@code SimpleDateFormat} lenient
+ * parsing only for a fixed set of numeric fields. This converter is the plan-time gate: it accepts
+ * a pattern only when every token maps to a field the native parser handles identically, and
+ * returns {@link Optional#empty()} otherwise so the whole {@code Calc} falls back to Flink's engine.
+ *
+ * <p>Accepted fields and their native specifiers:
+ * <ul>
+ * <li>{@code yyy} / {@code yyyy} → {@code %Y} (year)
+ * <li>{@code M} / {@code MM} → {@code %m} (month)
+ * <li>{@code d} / {@code dd} → {@code %d} (day of month)
+ * <li>{@code H} / {@code HH} → {@code %H} (hour of day)
+ * <li>{@code m} / {@code mm} → {@code %M} (minute)
+ * <li>{@code s} / {@code ss} → {@code %S} (second)
+ * </ul>
+ *
+ * <p>Non-alphabetic characters are literals (a literal {@code %} is emitted as {@code %%}).
+ * {@code SimpleDateFormat} single-quote escaping applies, including the doubled {@code ''} that
+ * denotes a literal quote. Every other ASCII letter and every unlisted run length forces a fall
+ * back, because Java reserves all letters and the omitted forms either depend on a locale or on the
+ * clock at parse time.
+ *
+ * <p>Adjacency rule: run length is erased by the translation ({@code M} and {@code MM} both become
+ * {@code %m}), yet the native parser reads each numeric field at a canonical width (year 4; month,
+ * day, hour, minute, second 2) while Java's lenient scan window equals the run length. When two
+ * numeric fields are adjacent with no literal separator, those two widths must agree, so the left
+ * field's run length must equal its canonical width; otherwise the pattern falls back to avoid a
+ * silent divergence (e.g. {@code yyyyMd} on {@code 20201010} yields month 1 in Java but month 10
+ * natively).
+ */
+public final class FlinkDateTimeFormatConverter {
+
+ private FlinkDateTimeFormatConverter() {
+ // utility class
+ }
+
+ /**
+ * Translates the given Java {@code SimpleDateFormat} pattern to the native {@code strftime}-style
+ * format string.
+ *
+ * @param javaPattern the Java date-time format pattern
+ * @return the translated native format string, or {@link Optional#empty()} if any part of the
+ * pattern is outside the natively supported surface
+ */
+ public static Optional<String> translate(String javaPattern) {
+ List<Token> tokens = scan(javaPattern);
+ if (tokens == null) {
Review Comment:
Added a `requireNonNull` in `cccb1acb`, but deliberately not the `Optional.empty()` behavior suggested here.
`Optional.empty()` is this converter's signal that the user wrote a pattern outside the native surface, and the whole Calc should fall back to Flink. That is a normal user-facing outcome. A null pattern is not that: it would mean our own converter never resolved a format, which is a plumbing bug. Mapping it to `empty()` would route the bug into the same silent fallback and hide it, so it fails fast instead. The same split already exists elsewhere in this path, where `isSupported` returns false for user-facing cases and `buildUnixTimestamp` throws `IllegalArgumentException` for plumbing bugs.
Null also cannot reach it today. The gate at `isUnixTimestampSupported` reads `javaFormat != null && FlinkDateTimeFormatConverter.translate(javaFormat).isPresent()`, and `&&` short-circuits. The other call site is inside the private `buildUnixTimestamp`, which the factory only reaches after `isSupported` returned true.
Covered by `testNullPatternRejectedRatherThanReportedUntranslatable`.
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "Tartarus0zm (via GitHub)" <gi...@apache.org>.
Tartarus0zm commented on PR #2448:
URL: https://github.com/apache/auron/pull/2448#issuecomment-5248906131
@weiqingy Regarding UNIX_TIMESTAMP, we still have two issues to resolve:
- The zero-argument case
- The timezone configuration parameter
It looks like the native UNIX_TIMESTAMP function will be implemented soon.
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #1863] Support native Flink UNIX_TIMESTAMP: converter integration [auron]
Posted by "Tartarus0zm (via GitHub)" <gi...@apache.org>.
Tartarus0zm merged PR #2448:
URL: https://github.com/apache/auron/pull/2448
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org