viirya commented on code in PR #299:
URL: https://github.com/apache/datafusion-comet/pull/299#discussion_r1576413764
##########
spark/src/main/scala/org/apache/spark/sql/comet/operators.scala:
##########
@@ -471,6 +488,9 @@ case class CometLocalLimitExec(
child: SparkPlan,
override val serializedPlanOpt: SerializedPlan)
extends CometUnaryExec {
+
+ override def outputPartitioning: Partitioning = child.outputPartitioning
Review Comment:
Because Spark's default `outputPartitioning` is `UnknownPartitioning`, if we
add `child.outputPartitioning` to `CometUnaryExec` as default, it will possibly
change `outputPartitioning` if we don't notice it.
Currently `CometExec` uses original Spark plan's `outputPartitioning` as
default which is safer option, I think. Except for the case that Spark
dynamically changes output partitioning during execution like AQE, it should be
correct because Comet doesn't change output partitioning from Spark.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
