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]