Copilot commented on code in PR #12442:
URL: https://github.com/apache/gluten/pull/12442#discussion_r3519015686
##########
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:
The Scaladoc for `notExecutable` says the child "declares" neither row nor
batch type, but `from.isNone` can also be produced by convention propagation
(e.g., a propagating node like `UnionExec` whose children have mixed
conventions). That makes the documentation misleading about the failure mode.
##########
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:
Same as the two-arg variant: the message says the plan "declares" neither
row nor batch type, but `isNone` can be derived. Prefer wording that describes
the observed convention state rather than attributing it to a declaration.
##########
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:
The exception message says the child plan "declares" neither row nor batch
type, but `from.isNone` can be a derived convention (e.g., from propagating
nodes with mixed child conventions). Rewording to "has no recognizable
convention" keeps the message accurate and aligns with the PR intent.
--
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]
