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 in this
PR's rework, now at the branch tip.
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 RuntimeException 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). This replaces the earlier silent
carrier passthrough through CAST / EXTRACT / comparison / CASE, which either
failed against the velox signatures or silently evaluated the integer carrier.
The rejection matrix and the whitelist are covered by the new planner tests,
and the rejected for
ms are additionally pinned by expected-reject e2e cases with native reference
outputs recorded.
--
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]
