Re: [PR] [SPARK-42746][SQL][FOLLOWUP] Correct the comments for SupportsOrderingWithinGroup and Mode [spark]

["cloud-fan (via GitHub)" <gi...@apache.org>]

cloud-fan commented on code in PR #49907:
URL: https://github.com/apache/spark/pull/49907#discussion_r1957266839


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Mode.scala:
##########
@@ -137,21 +137,20 @@ case class Mode(
     if (buffer.isEmpty) {
       return null
     }
-    /*
-      * The Mode class uses special collation awareness logic
-      *  to handle string data types with various collations.
-      *
-      * For string types that don't support binary equality,
-      * we create a new map where the keys are the collation keys of the original strings.
-      *
-      * Keys from the original map are aggregated based on the corresponding collation keys.
-      *  The groupMapReduce method groups the entries by collation key and maps each group
-      *  to a single value (the sum of the counts), and finally reduces the groups to a single map.
-      *
-      * The new map is then used in the rest of the Mode evaluation logic.
-      *
-      * It is expected to work for all simple and complex types with collated fields.
-      */
+
+    // The Mode class uses special collation awareness logic
+    //  to handle string data types with various collations.

Review Comment:
   ```suggestion
       // to handle string data types with various collations.
   ```



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Mode.scala:
##########
@@ -137,21 +137,20 @@ case class Mode(
     if (buffer.isEmpty) {
       return null
     }
-    /*
-      * The Mode class uses special collation awareness logic
-      *  to handle string data types with various collations.
-      *
-      * For string types that don't support binary equality,
-      * we create a new map where the keys are the collation keys of the original strings.
-      *
-      * Keys from the original map are aggregated based on the corresponding collation keys.
-      *  The groupMapReduce method groups the entries by collation key and maps each group
-      *  to a single value (the sum of the counts), and finally reduces the groups to a single map.
-      *
-      * The new map is then used in the rest of the Mode evaluation logic.
-      *
-      * It is expected to work for all simple and complex types with collated fields.
-      */
+
+    // The Mode class uses special collation awareness logic
+    //  to handle string data types with various collations.
+    //
+    // For string types that don't support binary equality,
+    // we create a new map where the keys are the collation keys of the original strings.
+    //
+    // Keys from the original map are aggregated based on the corresponding collation keys.
+    //  The groupMapReduce method groups the entries by collation key and maps each group

Review Comment:
   ditto



-- 
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: reviews-unsubscribe@spark.apache.org

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