metanil commented on code in PR #59760:
URL: https://github.com/apache/spark/pull/59760#discussion_r4432835534
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/physical/partitioning.scala:
##########
@@ -2415,6 +2419,23 @@ case class KeyedShuffleSpec(
private def argumentsAreColumns(transform: TransformExpression): Boolean =
transform.children.forall(_.isInstanceOf[Attribute])
+ /**
+ * Whether Spark can evaluate `transform` itself. An identity reduce
evaluates it on the identity
+ * side's partition keys, and a one-side shuffle on each row of the side it
shuffles. Only a
+ * `ScalarFunction` can be evaluated (see
`TransformExpression.resolvedFunction`), while a
+ * connector may bind its transform to any `BoundFunction`. Rebuilding the
transform over
+ * another key keeps its function, so asking this of the reported transform
answers it for the
+ * rebuilt one too.
+ *
+ * Resolving a `ScalarFunction` throws when Spark finds no method to call on
it, e.g. one that
+ * inherits `produceResult` from a base class. That counts as cannot
evaluate, not as a failure:
+ * the gates ask this of every candidate, including ones that end up not
being chosen.
+ */
+ private def canEvaluate(transform: TransformExpression): Boolean =
+ try transform.resolvedFunction.isDefined catch {
Review Comment:
Done! Both the `canEvaluate` doc and the `reducersBothWays` comment now name
the mixin forwarder case. `TestBucketFunction` has a real `produceResult` now,
so the scalar control doesn't depend on the forwarder.
--
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]
