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

    https://github.com/apache/spark/pull/7179#discussion_r33750308
  
    --- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
 ---
    @@ -42,4 +47,47 @@ class CodeGenerationSuite extends SparkFunSuite {
     
         futures.foreach(Await.result(_, 10.seconds))
       }
    +
    +  // Test GenerateOrdering for all common types. For each type, we 
construct random input rows that
    +  // contain two columns of that type, then for pairs of 
randomly-generated rows we check that
    +  // GenerateOrdering agrees with RowOrdering.
    +  (DataTypeTestUtils.atomicTypes ++ Set(NullType)).foreach { dataType =>
    +    test(s"GenerateOrdering with $dataType") {
    --- End diff --
    
    The nesting of the loops here is slightly misleading, because we'll always 
report a passed test for types where we don't have a data generator. We at 
least test that we're able to generate code for the ordering even if we don't 
actually execute that code. Maybe this is an okay trade-off, but it's a concern 
to watch out for.


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to