Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20911#discussion_r177641303
  
    --- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/complexTypesSuite.scala
 ---
    @@ -351,29 +389,65 @@ class ComplexTypesSuite extends PlanTest with 
ExpressionEvalHelper {
         // SPARK-23634.
         val arrayRel = relation
           .select(GetArrayItem(CreateArray(Seq('nullable_id, 'nullable_id + 
1L)), 0) as "a1")
    -      .groupBy($"a1")("1").analyze
    -    val arrayExpected = relation.select('nullable_id as 
"a1").groupBy($"a1")("1").analyze
    -    comparePlans(Optimizer execute arrayRel, arrayExpected)
    +      .groupBy($"a1")("1")
    +    val arrayExpected = relation.select('nullable_id as 
"a1").groupBy($"a1")("1")
    +    checkRule(arrayRel, arrayExpected)
     
         val mapRel = relation
           .select(GetMapValue(CreateMap(Seq("id", 'nullable_id)), "id") as 
"m1")
    -      .groupBy($"m1")("1").analyze
    +      .groupBy($"m1")("1")
         val mapExpected = relation
           .select('nullable_id as "m1")
    -      .groupBy($"m1")("1").analyze
    -    comparePlans(Optimizer execute mapRel, mapExpected)
    +      .groupBy($"m1")("1")
    +    checkRule(mapRel, mapExpected)
       }
     
       test("SPARK-23500: Ensure that aggregation expressions are not 
simplified") {
         // Make sure that aggregation exprs are correctly ignored. Maps can't 
be used in
         // grouping exprs so aren't tested here.
         val structAggRel = relation.groupBy(
           CreateNamedStruct(Seq("att1", 'nullable_id)))(
    -      GetStructField(CreateNamedStruct(Seq("att1", 'nullable_id)), 0, 
None)).analyze
    -    comparePlans(Optimizer execute structAggRel, structAggRel)
    +      GetStructField(CreateNamedStruct(Seq("att1", 'nullable_id)), 0, 
None))
    +    checkRule(structAggRel, structAggRel)
     
         val arrayAggRel = relation.groupBy(
    -      
CreateArray(Seq('nullable_id)))(GetArrayItem(CreateArray(Seq('nullable_id)), 
0)).analyze
    -    comparePlans(Optimizer execute arrayAggRel, arrayAggRel)
    +      
CreateArray(Seq('nullable_id)))(GetArrayItem(CreateArray(Seq('nullable_id)), 0))
    +    checkRule(arrayAggRel, arrayAggRel)
    +  }
    +
    +  test("SPARK-23500: do not simplify maps in Aggregate expressions") {
    +    // This could be done if we had a more complex rule that checks that
    --- End diff --
    
    shall we merge it with the previous test case?


---

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

Reply via email to