HeartSaVioR commented on a change in pull request #31580:
URL: https://github.com/apache/spark/pull/31580#discussion_r579584356



##########
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:
       Just 2 cents, I'd feel more natural to change the null ordering 
"optional" in SortOrder. It'd be more intuitive on the behavior we want, "if 
null ordering is not presented, default null ordering of SortDirection is 
taken". Callers of SortOrder has to resolve the null ordering but then we no 
longer delegate the implementations to do that instead.
   
   Each implementation can still simply call the method to get the default null 
ordering and provide it so no strong voice. Just would like to think out loud 
what'd be the clearer.




----------------------------------------------------------------
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:
[email protected]



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

Reply via email to