sunshone commented on code in PR #13470:
URL: https://github.com/apache/gluten/pull/13470#discussion_r4412653380
##########
gluten-flink/runtime/src/main/java/org/apache/gluten/util/LogicalTypeConverter.java:
##########
@@ -117,6 +118,13 @@ private interface VLTypeConverter {
Map.entry(
TinyIntType.class,
logicalType -> new
io.github.zhztheplayer.velox4j.type.TinyIntType()),
+ // Map Flink's `TIME` type to the velox `INTEGER` type: Flink
carries a
+ // TIME(p) field in RowData as an int of milliseconds of day, so
INTEGER
+ // is the isomorphic carrier (the exact RowData representation);
velox
+ // itself has no TIME type.
+ // TODO: support FLINK TIME type
+ Map.entry(
+ TimeType.class, logicalType -> new
io.github.zhztheplayer.velox4j.type.IntegerType()),
Review Comment:
Thanks a lot for flagging this — addressed by explicit rejection (gluten
commit 50e6b1261).
Supporting the wider TIME expression surface
(formatting/EXTRACT/arithmetic/CAST on TIME semantics) would require a whole
new TIME function family in velox, out of scope for this change; instead the
planner now fails fast: RexNodeConverter rejects any RexCall (other than the
LOCALTIME direct projection) whose own type or any direct operand is
TIME-typed, and a bare TIME literal, with an explicit message naming the
operator and the supported surface ("gluten-flink supports TIME-typed values
only as the LOCALTIME direct projection on the INTEGER milliseconds-of-day
carrier"). Whitelisted: the LOCALTIME projection itself and TIME-typed
InputRef/FieldAccess (direct carrier passthrough). Unit tests G1-G4 cover the
rejection matrix (CAST/EXTRACT/comparison/CASE) and G5 the whitelist; the e2e
dual-run adds three expected-reject cases (032 CAST / 034 EXTRACT / 035
comparison: GFV planning-time rejection with the guard message, native
reference output recorded), so the silently-integer-car
rier path is no longer reachable. No checked-in test used TIME-typed
expressions before, so nothing breaks.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
