advancedxy commented on code in PR #88:
URL:
https://github.com/apache/arrow-datafusion-comet/pull/88#discussion_r1500114619
##########
spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala:
##########
@@ -320,6 +320,20 @@ class CometSparkSessionExtensions
c
}
+ case s: TakeOrderedAndProjectExec
+ if isCometNative(s.child) && isCometOperatorEnabled(conf,
"takeOrderedAndProjectExec")
+ && isCometShuffleEnabled(conf) &&
+ CometTakeOrderedAndProjectExec.isSupported(s.projectList,
s.sortOrder, s.child) =>
+ // TODO: support offset for Spark 3.4
Review Comment:
One minor comment: this might be a correctness issue for Spark 3.4 with
`offset > 0` as we simply ignore the possible `offset` field.
One possible way to address that would be that define a `getOffset` method
in ` ShimCometSparkSessionExtensions` to access `offset` field via reflection.
Of course it should be addressed in another PR and other `xxLimitExec` too.
--
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]
