dwsmith1983 commented on code in PR #6270:
URL: https://github.com/apache/datafusion-comet/pull/6270#discussion_r4266613375


##########
spark/src/main/scala/org/apache/spark/sql/comet/CometNativeScanExec.scala:
##########
@@ -285,14 +285,13 @@ case class CometNativeScanExec(
   }
 
   override def doCanonicalize(): CometNativeScanExec = {
-    // Canonicalize originalPlan but strip its DPP partition filters.
-    // originalPlan carries column selection and schema info needed for
-    // equals/hashCode. But its partitionFilters may have stale DPP
-    // expressions (e.g., SABs not yet converted to TrueLiteral) that
-    // would prevent exchange reuse between otherwise-identical scans.
+    // Canonicalize originalPlan without any of its DPP partition filters. 
originalPlan carries
+    // column selection and schema info needed for equals/hashCode, but its 
DPP filters are a stale
+    // copy that no DPP rule rewrites. The top-level partitionFilters below 
carry the scan's DPP
+    // identity.
     val canonOriginal = if (originalPlan != null) {
       val stripped = originalPlan.copy(partitionFilters =
-        
CometScanUtils.filterUnusedDynamicPruningExpressions(originalPlan.partitionFilters))
+        
originalPlan.partitionFilters.filterNot(_.isInstanceOf[DynamicPruningExpression]))

Review Comment:
   Moved it to `CometScanUtils.canonicalizeOriginalPlan`, next to 
`filterUnusedDynamicPruningExpressions`. It takes the nullable 
`FileSourceScanExec` and returns its canonical copy without any DPP filters. 
`CometNativeScanExec` and `CometDeltaNativeScanExec` both call it, the 
explanation now sits once on the helper, and `adding_a_new_operator.md` tells a 
new scan to call both helpers instead of copying the block.
   



-- 
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]

Reply via email to