jiwen624 commented on code in PR #57272:
URL: https://github.com/apache/spark/pull/57272#discussion_r3591157939


##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/CollatedString.java:
##########
@@ -17,34 +17,37 @@
 
 package org.apache.spark.sql.catalyst.expressions;
 
+import java.util.Arrays;
+
 /**
  * A DataSketches ItemsSketch item for non-binary collated strings 
(SPARK-58069).
  * <p>
  * Equality and hashing are driven solely by the collation {@code key}, so 
that collation-equal
  * strings (e.g. {@code 'HELLO'} and {@code 'hello'} under {@code UTF8_LCASE}) 
are counted as a
  * single item. The {@code original} field retains an actual input value to 
return in the result,
  * mirroring how {@code mode()} returns a real value rather than the 
normalized collation key.
+ * <p>
+ * The {@code key} is the raw collation sort-key bytes (SPARK-58096). ICU sort 
keys are arbitrary
+ * bytes, not valid UTF-8, so decoding them to a {@code String} is lossy: two 
collation-distinct
+ * values whose keys differ only within invalid-byte regions would decode to 
the same {@code String}
+ * and be incorrectly merged. Keying on the bytes directly avoids that 
over-merge.
  */
 public class CollatedString {
-    private final String key;
+    private final byte[] key;
     private final String original;
 
-    public CollatedString(String key, String original) {
+    public CollatedString(byte[] key, String original) {
         this.key = key;
         this.original = original;
     }
 
-    public String key() {

Review Comment:
   Dropped the unused key() accessor: nothing reads it, and exposing the raw 
sort-key byte[] leads to a risk of it being mutated externally.



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

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