sunchao commented on code in PR #5614:
URL: https://github.com/apache/datafusion-comet/pull/5614#discussion_r3910237757
##########
spark/src/main/scala/org/apache/comet/serde/arrays.scala:
##########
@@ -924,4 +924,38 @@ object CometArraySort extends
CometCodegenDispatch[ArraySort]
object CometZipWith extends CometCodegenDispatch[ZipWith]
-object CometSequence extends CometCodegenDispatch[Sequence]
+object CometSequence extends CometExpressionSerde[Sequence] with
CodegenDispatchFallback {
+
+ private val temporalUnsupportedReason =
+ "date and timestamp element types run through the JVM codegen dispatcher"
+
+ override def getSupportLevel(expr: Sequence): SupportLevel =
expr.start.dataType match {
+ case ByteType | ShortType | IntegerType | LongType => Compatible()
+ case DateType | TimestampType | TimestampNTZType =>
+ // Temporal sequences step through timezone/DST/legacy-calendar
arithmetic
+ // (https://github.com/apache/datafusion-comet/issues/5349), so they
stay on the JVM
+ // codegen dispatcher.
+ Unsupported(Some(temporalUnsupportedReason))
+ case other =>
+ Unsupported(Some(s"sequence with element type $other is not supported
natively"))
+ }
+
+ override def getUnsupportedReasons(): Seq[String] =
Seq(temporalUnsupportedReason)
+
+ override def convert(
+ expr: Sequence,
+ inputs: Seq[Attribute],
+ binding: Boolean): Option[ExprOuterClass.Expr] = {
+ val startExprProto = exprToProto(expr.start, inputs, binding)
+ val stopExprProto = exprToProto(expr.stop, inputs, binding)
+ // With no step argument the native kernel computes Spark's per-row
default,
+ // `start <= stop ? 1 : -1`, which cannot be expressed as a plan-time
literal.
+ val argProtos = Seq(startExprProto, stopExprProto) ++
+ expr.stepOpt.map(exprToProto(_, inputs, binding))
+ scalarFunctionExprToProtoWithReturnType(
Review Comment:
[P2] Preserve null short-circuiting before evaluating later arguments
Could this lowering preserve `Sequence`'s left-to-right null guards? For a
Parquet table `t(s INT, k INT)` containing `(NULL, -1)` and `(1, 1)`, consider
`SELECT sequence(s, size(sequence(1, 5, k))) FROM t`. Spark returns `NULL` for
the first row without evaluating the inner sequence, and `[1,2,3,4,5]` for the
second. Here both sequences become scalar UDFs, whose arguments DataFusion
evaluates over the batch before calling the outer kernel. The inner
`sequence(1, 5, -1)` therefore throws before the outer `row_is_null` check can
discard that row. The previous dispatcher kept the whole expression tree inside
Spark's guarded evaluation. Could we retain those guards or dispatch such
shapes, and add a composed null/error regression case?
--
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]
