Skip to content

[GLUTEN-13471][FLINK] Support TO_DATE / TO_TIMESTAMP / TO_TIMESTAMP_LTZ offload to velox flinksql functions - #13473

Open
TianJialong wants to merge 7 commits into
apache:mainfrom
TianJialong:main
Open

TianJialong wants to merge 7 commits into
apache:mainfrom
TianJialong:main

Conversation

@TianJialong

@TianJialong TianJialong commented Oct 27, 2026 •

Copy link
Copy Markdown

What changes are proposed in this pull request?

Fixes #13471

Maps the temporal string/number conversion expressions to their velox flinksql implementations (companion velox PR bigo-sg/velox#94, velox4j PR bigo-sg/velox4j#75):

  • RexCallConverterFactory: TO_DATE / TO_TIMESTAMP / TO_TIMESTAMP_LTZ entries — plain name mappings; both arities of the first two are covered by the flinksql signatures, TO_TIMESTAMP_LTZ's numeric argument types bind through the same registration.
  • RexNodeConverter: DATE literals convert to the epoch-day IntegerValue — Flink folds constant TO_DATE calls in the planner, and the plan used to die at conversion with the DATE literal unsupported. TIMESTAMP_WITH_LOCAL_TIME_ZONE literals share the TIMESTAMP conversion body (TimestampString.getMillisSinceEpoch → boxed-Long millis, with a null guard), so folded TO_TIMESTAMP / TO_TIMESTAMP_LTZ literals reach velox. NULL literal branches emit a null variant (a velox4j serialization limitation currently drops typed NULL literals at the bridge; NULL rows arrive from data inputs instead).
  • RexNodeConverter: identity-cast elimination — the planner emits CAST(literal AS T) around every folded literal whose nullability differs from the sink's, and velox rejects same-type casts, so isSameTypeIgnoringNullability (recursive type equality ignoring nullability only; ANY never elided; composite component types must match) passes the operand through unchanged.
  • RexNodeConverterTest: covers the two literal channels, the identity-cast behavior across scalar and composite types, and the NULL literal branches.

How was this patch tested?

  • Planner-level: RexNodeConverterTest — DATE literal converts to the epoch-day IntegerValue, LTZ literal to epoch millis; identity CAST elided on nullability-only casts for scalar and composite types (ARRAY/ROW/MAP casts with different component types stay real casts); NULL literal branches.
  • Execution-level (embedded gluten stack): DateTimeFunctionsTest in org.apache.gluten.table.runtime.stream.custom — both arities of to_date (incl. constant-format folding through the DATE literal channel), to_timestamp (SMART clamping, 24:00 rollover, fraction flooring, column/constant formats), to_timestamp_ltz (precision 0/3 unit semantics, epoch-domain bounds, double/decimal inputs), NULL propagation on all forms, and CASE/WHERE combinations.
  • End-to-end dual-run on Flink 1.19.2: 32 SQL scenarios across the family — both arities, constant vs column formats, NULL propagation, literal folding for DATE and LTZ, invalid-input and out-of-range classes — with 29 exact matches and 3 shared job-failure classes (illegal format, year overflow, unsupported LTZ precision: identical failures on both stacks), zero mismatches.

@github-actions github-actions Bot added the FLINK label Oct 27, 2026
@TianJialong

Copy link
Copy Markdown
Author

@KevinyhZou @lgbo-ustc @philo-he @ggjh-159 Would you mind reviewing this PR?

@lgbo-ustc lgbo-ustc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR depends on bigo-sg/velox#94, but the Flink build still pins a Velox4j revision whose Velox dependency predates those functions. Please update the Velox and Velox4j references so the new offload mappings are built and tested against a version that contains the required implementations.

Map.entry(
"REGEXP_EXTRACT", Arrays.asList(() -> new DefaultRexCallConverter("regexp_extract"))),
// Direct name mapping: both arities are covered by the flinksql to_date signatures.
Map.entry("TO_DATE", Arrays.asList(() -> new DefaultRexCallConverter("to_date"))),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] These new TO_DATE / TO_TIMESTAMP / TO_TIMESTAMP_LTZ mappings require the flinksql registrations introduced by bigo-sg/velox#94. However, this PR does not change .github/workflows/flink.yml, which still checks out bigo-sg/velox4j at 26c7715278e6f6795084f6334998cb2ce382f7aa (lines 90-92). That Velox4j revision reads src/main/cpp/velox-ref.txt and pins Velox to aff6baab84d650cc22dd8b7cf1ab55f0ea52d929, which is 22 commits behind the #94 head (6aa6b617f45365bde843dcdf4546a7eb602e2f17) and has no TemporalFunctionsRegistration.cpp. Please update the Velox4j Velox ref to a commit containing #94, publish the corresponding Velox4j revision, and pin that Velox4j revision in this PRs Flink workflow. Otherwise the new planner expressions will reach a native engine without these functions and fail at execution; the current CI build cannot validate the feature.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgbo-ustc Thanks for the review! Pushed 157cb39 (Additionally, some test cases were added), please check.

@github-actions github-actions Bot added the INFRA label Oct 29, 2026
@TianJialong
TianJialong requested a review from lgbo-ustc October 29, 2026 03:46

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FLINK][VL] Support the TO_DATE / TO_TIMESTAMP / TO_TIMESTAMP_LTZ temporal expressions

2 participants