LuciferYang commented on code in PR #12442:
URL: https://github.com/apache/gluten/pull/12442#discussion_r3519087617


##########
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/transition/Transition.scala:
##########
@@ -65,6 +65,26 @@ object Transition {
       s"No viable transition to [$required] found for plan: 
${plan.simpleString(maxFields)}")
   }
 
+  /**
+   * Raised when `plan` has a child that declares no recognizable row type and 
no recognizable batch
+   * type. Such a child cannot be executed and has no `from` convention to 
search from, so callers
+   * should raise this at planning time instead of accepting the child and 
letting the failure
+   * surface later at task execution.
+   */

Review Comment:
   Good catch — reworded to describe the observed convention state ("has no 
recognizable row type or batch type") instead of attributing it to a 
declaration. Amended in the latest push.



##########
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/transition/Transition.scala:
##########
@@ -65,6 +65,26 @@ object Transition {
       s"No viable transition to [$required] found for plan: 
${plan.simpleString(maxFields)}")
   }
 
+  /**
+   * Raised when `plan` has a child that declares no recognizable row type and 
no recognizable batch
+   * type. Such a child cannot be executed and has no `from` convention to 
search from, so callers
+   * should raise this at planning time instead of accepting the child and 
letting the failure
+   * surface later at task execution.
+   */
+  def notExecutable(plan: SparkPlan, child: SparkPlan): GlutenException = {
+    new GlutenException(
+      "Child plan declares neither a row type nor a batch type, so no 
transition can be " +
+        "inserted for it. Parent: " +
+        s"${plan.simpleString(maxFields)}. Child: 
${child.simpleString(maxFields)}.")
+  }

Review Comment:
   Good catch — reworded to describe the observed convention state ("has no 
recognizable row type or batch type") instead of attributing it to a 
declaration. Amended in the latest push.



##########
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/transition/Transition.scala:
##########
@@ -65,6 +65,26 @@ object Transition {
       s"No viable transition to [$required] found for plan: 
${plan.simpleString(maxFields)}")
   }
 
+  /**
+   * Raised when `plan` has a child that declares no recognizable row type and 
no recognizable batch
+   * type. Such a child cannot be executed and has no `from` convention to 
search from, so callers
+   * should raise this at planning time instead of accepting the child and 
letting the failure
+   * surface later at task execution.
+   */
+  def notExecutable(plan: SparkPlan, child: SparkPlan): GlutenException = {
+    new GlutenException(
+      "Child plan declares neither a row type nor a batch type, so no 
transition can be " +
+        "inserted for it. Parent: " +
+        s"${plan.simpleString(maxFields)}. Child: 
${child.simpleString(maxFields)}.")
+  }
+
+  /** Single-argument variant for the root-plan case. See 
[[notExecutable(plan, child)]]. */
+  def notExecutable(plan: SparkPlan): GlutenException = {
+    new GlutenException(
+      "Plan declares neither a row type nor a batch type, so no transition can 
be inserted for " +
+        s"it. Plan: ${plan.simpleString(maxFields)}.")
+  }

Review Comment:
   Good catch — reworded to describe the observed convention state ("has no 
recognizable row type or batch type") instead of attributing it to a 
declaration. Amended in the latest push.



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