andygrove commented on code in PR #5236:
URL: https://github.com/apache/datafusion-comet/pull/5236#discussion_r3713471894


##########
spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala:
##########
@@ -741,6 +759,57 @@ case class CometExecRule(session: SparkSession)
     None
   }
 
+  /**
+   * Lift fallback reasons recorded on `op`'s expression trees onto `op` 
itself.
+   *
+   * Extended explain output only walks plan nodes 
(`ExtendedExplainInfo.sortup` follows
+   * `children` / `innerChildren`, never `expressions`), so a reason tagged on 
an expression is
+   * invisible unless something lifts it onto the enclosing operator. This 
mirrors what
+   * [[rollUpInfoMessages]] already does for the informational tags, and 
replaces the roll-up that
+   * used to be hand-written at every serde call site (see
+   * https://github.com/apache/datafusion-comet/issues/5230).
+   *
+   * Only child *expressions* are collected, not child operators: reasons on a 
child operator are
+   * already reachable by the explain traversal via `children`.
+   *
+   * Called only when `op` was left in the Spark plan, which scopes the 
roll-up to the operator
+   * that actually failed conversion. That matters because some expression 
instances
+   * (`AttributeReference`s, DPP subquery expressions) are shared across 
operators, so an unscoped
+   * roll-up could surface one expression's reason under several unrelated 
operators.
+   */
+  private def rollUpFallbackReasons(op: SparkPlan): Unit = {
+    val reasons = op.expressions
+      .flatMap(_.collect { case e: Expression => e })
+      .flatMap(_.getTagValue(CometExplainInfo.FALLBACK_REASONS))
+      .flatten
+      .toSet
+    if (reasons.nonEmpty) {
+      withFallbackReasons(op, reasons)

Review Comment:
   We roll up the expression fallback reasons to the operator once in the 
framework instead of hand-coding it for every single operator



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