You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/02/19 13:50:56 UTC

[GitHub] [spark] cloud-fan commented on a change in pull request #31580: [SPARK-34457][SQL] DataSource V2: Add default null ordering to SortDirection

cloud-fan commented on a change in pull request #31580:
URL: https://github.com/apache/spark/pull/31580#discussion_r579199061



##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/SortDirection.java
##########
@@ -19,14 +19,33 @@
 
 import org.apache.spark.annotation.Experimental;
 
+import static org.apache.spark.sql.connector.expressions.NullOrdering.NULLS_FIRST;
+import static org.apache.spark.sql.connector.expressions.NullOrdering.NULLS_LAST;
+
 /**
  * A sort direction used in sorting expressions.
+ * <p>
+ * Each direction has a default null ordering that is implied if no null ordering is specified
+ * explicitly.
  *
  * @since 3.2.0
  */
 @Experimental
 public enum SortDirection {
-  ASCENDING, DESCENDING;
+  ASCENDING(NULLS_FIRST), DESCENDING(NULLS_LAST);
+
+  private final NullOrdering defaultNullOrdering;
+
+  SortDirection(NullOrdering defaultNullOrdering) {
+    this.defaultNullOrdering = defaultNullOrdering;
+  }
+
+  /**
+   * Returns the default null ordering to use if no null ordering is specified explicitly.
+   */
+  public NullOrdering defaultNullOrdering() {

Review comment:
       This makes sense, because the catalyst `SortDirection` also has a default null ordering.
   
   Shall we further follow catalyst and add an overload of `Expressions.sort` which doesn't take `NullOrdering` and infer null ordering from `SortDirection`?




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

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



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