aokolnychyi commented on code in PR #37749:
URL: https://github.com/apache/spark/pull/37749#discussion_r961211655


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/V2ExpressionUtils.scala:
##########
@@ -105,18 +105,27 @@ object V2ExpressionUtils extends SQLConfHelper with 
Logging {
           TransformExpression(bound, resolvedRefs, Some(numBuckets))
         }
       }
-    case NamedTransform(name, refs)
-        if refs.length == 1 && refs.forall(_.isInstanceOf[NamedReference]) =>

Review Comment:
   I added tests but they seem to work only by accident as `satisfies0` in 
`KeyGroupedPartitioning` returns false and triggers a shuffle. We probably need 
to adapt the existing logic to be more reliable and explicit.
   
   I see two solutions:
   - Explicitly check we have flat transforms with one child ref in 
`satisfies0` in `KeyGroupedPartitioning`.
   - Don't construct `KeyGroupedPartitioning` unless we have flat transforms 
with one child ref.
   
   I am inclined to go with the first option but let me know if you have a 
better idea.



-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to