andygrove commented on code in PR #6135:
URL: https://github.com/apache/datafusion-comet/pull/6135#discussion_r4156049996
##########
spark/src/main/scala/org/apache/comet/rules/RevertNativeForTransitionHeavyStages.scala:
##########
@@ -198,7 +198,28 @@ case class RevertNativeForTransitionHeavyStages(session:
SparkSession, wholePlan
case local: CometLocalTopKExec => local.child
case cometExec: CometExec =>
if (cometExec.originalPlan.children.size == cometExec.children.size) {
- cometExec.originalPlan.withNewChildren(cometExec.children)
+ val originalWithCurrentExpressions = cometExec match {
Review Comment:
This change and its two tests are not part of the `allowIncompatible`
cleanup. The `CometNativeScanExec` branch and the
`RevertNativeForTransitionHeavyStagesSuite` test are the same fix as #6137
(which closes #5486), so whichever PR lands second will conflict on this hunk.
The `CometIcebergNativeScanExec` branch is also only in this PR, so it will not
be reviewed or merged with the V1 fix. The description here still says the PR
does not change behavior and adds no tests. Could we drop `cefe845a1` from this
PR and move the Iceberg case into #6137? That keeps this PR a pure
no-behavior-change cleanup and lets the DPP fix be reviewed with its own
description.
--
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]
