Copilot commented on code in PR #13202:
URL: https://github.com/apache/gluten/pull/13202#discussion_r4190747091
##########
gluten-substrait/src/main/scala/org/apache/gluten/expression/WindowFunctionsBuilder.scala:
##########
@@ -63,4 +66,21 @@ object WindowFunctionsBuilder {
w
}
}
+
+ // Whether WindowFunctionNode can convert the frame bound: a special or
non-foldable bound, or
+ // a literal that parses as the long offset it sends to the native engine.
+ def isLongOffset(bound: Expression): Boolean = bound match {
+ case _: SpecialFrameBoundary => true
+ case b if !b.foldable => true
+ case b =>
Try(java.lang.Long.parseLong(String.valueOf(b.eval(EmptyRow)))).isSuccess
+ }
+
+ // A literal RANGE frame bound is offloaded as a pre-computed boundary
column, which only works
+ // for an integral bound; any other literal bound makes the window fall back.
+ def checkRangeFrameLiteralBound(bound: Expression): Unit = bound.dataType
match {
+ case ByteType | ShortType | IntegerType | LongType =>
+ case other =>
+ throw new GlutenNotSupportException(
+ s"Only integral literal bounds are supported for a RANGE frame, got
${other.simpleString}")
+ }
Review Comment:
`checkRangeFrameLiteralBound` accepts every integral-typed literal without
checking that it actually converts. In particular, a null `LongType` bound
passes this JVM-side check, while `WindowFunctionNode.parseOffset` (and the new
unit test) rejects it. With native validation disabled, Velox/Bolt therefore
still offload such a frame and throw during real plan serialization instead of
falling back. Apply `isLongOffset(bound)` here as well before accepting the
integral type, so the support check and conversion remain consistent.
--
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]
