sunchao commented on code in PR #5688:
URL: https://github.com/apache/datafusion-comet/pull/5688#discussion_r3937467919
##########
spark/src/main/scala/org/apache/comet/serde/arrays.scala:
##########
@@ -814,10 +814,14 @@ object CometArrayPosition extends
CometExpressionSerde[ArrayPosition] with Array
}
}
-object CometArraysZip extends CometExpressionSerde[ArraysZip] {
+object CometArraysZip extends CometExpressionSerde[ArraysZip] with
CodegenDispatchFallback {
Review Comment:
### Performance
[P2] Could you add a matched microbenchmark for the newly dispatched
arrays-of-maps path before enabling it by default? This marker changes existing
queries from Spark projection fallback to Spark-generated evaluation inside the
JVM bridge. That path still evaluates `ArraysZip` row by row and additionally
imports the input and materializes the nested Arrow output. Avoiding the
surrounding projection fallback can pay for those costs, but the two
correctness queries do not establish when it does.
Please compare dispatch enabled with the existing dispatcher-disabled
fallback on identical nonconstant column inputs, including a map-only
projection and the mixed native projection, small and multi-batch inputs, and
small versus larger maps/arrays. Verify the same results and include warm
steady-state timings with the batch size and data shape. I am not asserting a
measured slowdown. The missing comparison is the evidence needed to assess the
default-path performance tradeoff.
--
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]
