Re: [PR] feat(sort): add BINARY type support [texera]

["zuozhiw (via GitHub)" <gi...@apache.org>]

zuozhiw commented on code in PR #4014:
URL: https://github.com/apache/texera/pull/4014#discussion_r2485563501


##########
common/workflow-operator/src/main/scala/org/apache/amber/operator/sort/StableMergeSortOpExec.scala:
##########
@@ -225,9 +225,21 @@ class StableMergeSortOpExec(descString: String) extends OperatorExecutor {
   /**
     * Compare two non-null values using their attribute type.
     *
-    * For DOUBLE:
-    *  - Uses java.lang.Double.compare (orders -Inf < ... < +Inf < NaN).
-    *  - Callers if desired should define how NaN interacts with ASC/DESC and null policy.
+    * Type semantics:
+    *  - INTEGER, LONG: numeric ascending via Java primitive compares.
+    *  - DOUBLE: java.lang.Double.compare (orders -Inf < ... < +Inf < NaN).
+    *  - BOOLEAN: false < true.
+    *  - TIMESTAMP: java.sql.Timestamp#compareTo.
+    *  - STRING: String#compareTo (UTF-16, lexicographic).
+    *  - BINARY: unsigned lexicographic order over byte arrays:
+    *        - Compare byte-by-byte treating each as 0..255 (mask 0xff).
+    *        - The first differing byte decides the order.
+    *        - If all compared bytes are equal, the shorter array sorts first.
+    *        - Example: [] < [0x00] < [0x00,0x00] < [0x00,0x01] < [0x7F] < [0x80] < [0xFF].
+    *
+    * Implementation note:
+    *   -  BINARY uses [[java.util.Arrays.compareUnsigned]] (JDK 9+). For JDK 8,
+    *     replace with a manual unsigned lexicographic comparator.

Review Comment:
   probably no need to mention jdk8 in comment, do we have jdk8 compat requirements?



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

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

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