Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9409#discussion_r44215590
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Utils.scala
 ---
    @@ -54,10 +54,14 @@ object Utils {
                 mode = aggregate.Complete,
                 isDistinct = false)
     
    -        // We do not support multiple COUNT DISTINCT columns for now.
    -        case expressions.CountDistinct(children) if children.length == 1 =>
    +        case expressions.CountDistinct(children) =>
    +          val child = if (children.size > 1) {
    +            DropAnyNull(CreateStruct(children))
    --- End diff --
    
    oh, yes. Based on 
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala#L240-L266,
 we can compare two struct values. Looks like only array and map types are not 
handled there. So, I think we can visit all data types of a struct and if it 
does not have array or map, we can use new agg code path. Can you update 
Utils.scala? I am thinking about if an array or a map appear in the grouping 
expressions, we throw an analysis error and say it is not allowed right now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

Reply via email to