parthchandra commented on code in PR #386:
URL: https://github.com/apache/datafusion-comet/pull/386#discussion_r1591291729
##########
spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala:
##########
@@ -1042,17 +1042,36 @@ object CometSparkSessionExtensions extends Logging {
* The node with information (if any) attached
*/
def withInfo[T <: TreeNode[_]](node: T, info: String, exprs: T*): T = {
- val exprInfo = exprs
- .flatMap { e => Seq(e.getTagValue(CometExplainInfo.EXTENSION_INFO)) }
- .flatten
- .mkString("\n")
- if (info != null && info.nonEmpty && exprInfo.nonEmpty) {
- node.setTagValue(CometExplainInfo.EXTENSION_INFO, Seq(exprInfo,
info).mkString("\n"))
- } else if (exprInfo.nonEmpty) {
- node.setTagValue(CometExplainInfo.EXTENSION_INFO, exprInfo)
- } else if (info != null && info.nonEmpty) {
- node.setTagValue(CometExplainInfo.EXTENSION_INFO, info)
+ // support existing approach of passing in multiple infos in a
newline-delimited string
+ val infoSet = if (info == null || info.isEmpty) {
+ Set.empty[String]
+ } else {
+ info.split("\n").toSet
}
+ withInfos(node, infoSet, exprs: _*)
+ }
+
+ /**
+ * Attaches explain information to a TreeNode, rolling up the corresponding
information tags
+ * from any child nodes. For now, we are using this to attach the reasons
why certain Spark
+ * operators or expressions are disabled.
+ *
+ * @param node
+ * The node to attach the explain information to. Typically a SparkPlan
+ * @param info
+ * Information text. May contain zero or more strings. If not provided,
then only information
+ * from child nodes will be included.
+ * @param exprs
+ * Child nodes. Information attached in these nodes will be be included in
the information
+ * attached to @node
+ * @tparam T
+ * The type of the TreeNode. Typically SparkPlan, AggregateExpression, or
Expression
+ * @return
+ * The node with information (if any) attached
+ */
+ def withInfos[T <: TreeNode[_]](node: T, info: Set[String], exprs: T*): T = {
+ val exprInfo =
exprs.flatMap(_.getTagValue(CometExplainInfo.EXTENSION_INFO)).flatten.toSet
+ node.setTagValue(CometExplainInfo.EXTENSION_INFO, exprInfo ++ info)
Review Comment:
Ah. So this is what I was missing when I implemented the previous version.
--
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]
