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

    https://github.com/apache/spark/pull/14842#discussion_r78267899
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala
 ---
    @@ -71,12 +95,34 @@ case class SortPrefix(child: SortOrder) extends 
UnaryExpression {
     
       val nullValue = child.child.dataType match {
         case BooleanType | DateType | TimestampType | _: IntegralType =>
    -      Long.MinValue
    +      if ((child.isAscending && child.nullOrdering == NullFirst) ||
    +        (!child.isAscending && child.nullOrdering == NullLast)) {
    +          Long.MinValue
    --- End diff --
    
    Whether we return a smallest or biggest value for null depending on the 
combination of the sorting direction and null ordering, because our prefix 
comparators simply compare 2 values. For DESC, they swap the input parameters 
(aPrefix, bPrefix) to compare. 
    
    For example, in ASC NULLS LAST case, we want the NULL rows be placed to the 
last, which means the comparator needs to think the NULL value is the largest. 
While in DESC NULLS FIRST case, we want the NULL rows be placed to the first, 
which means the comparator needs to think the NULL value is also the largest. 


---
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