uros-b commented on code in PR #57272:
URL: https://github.com/apache/spark/pull/57272#discussion_r3683504836


##########
sql/core/src/test/scala/org/apache/spark/sql/ApproxTopKSuite.scala:
##########
@@ -310,6 +310,52 @@ class ApproxTopKSuite extends SharedSparkSession {
     }
   }
 
+  // scalastyle:off nonascii
+  test("SPARK-58096: approx_top_k keys on raw sort-key bytes, not a 
lossy-decoded String, " +
+    "so ICU-collation-distinct non-ASCII values are not over-merged") {
+    // U+4E14, U+4E15 and U+4E16 are distinct under UNICODE_CI, but their ICU 
sort keys are
+    // arbitrary bytes that decode to the same String via a lossy UTF-8 
conversion. Keying the
+    // sketch on that decoded String would collapse them into a single item 
with an inflated
+    // count; keying on the raw sort-key bytes keeps them separate.
+    val res = sql(
+      s"""SELECT approx_top_k(c, 3)

Review Comment:
   ```suggestion
         """SELECT approx_top_k(c, 3)
   ```



##########
sql/core/src/test/scala/org/apache/spark/sql/ApproxTopKSuite.scala:
##########
@@ -310,6 +310,52 @@ class ApproxTopKSuite extends SharedSparkSession {
     }
   }
 
+  // scalastyle:off nonascii
+  test("SPARK-58096: approx_top_k keys on raw sort-key bytes, not a 
lossy-decoded String, " +
+    "so ICU-collation-distinct non-ASCII values are not over-merged") {
+    // U+4E14, U+4E15 and U+4E16 are distinct under UNICODE_CI, but their ICU 
sort keys are
+    // arbitrary bytes that decode to the same String via a lossy UTF-8 
conversion. Keying the
+    // sketch on that decoded String would collapse them into a single item 
with an inflated
+    // count; keying on the raw sort-key bytes keeps them separate.
+    val res = sql(
+      s"""SELECT approx_top_k(c, 3)
+         |FROM (SELECT CAST(col AS STRING COLLATE UNICODE_CI) AS c
+         |      FROM VALUES ('且'), ('且'), ('且'),
+         |                  ('丕'), ('丕'), ('世') AS t(col))
+         |""".stripMargin)
+    checkAnswer(res, Row(Seq(Row("且", 3), Row("丕", 2), Row("世", 1))))
+  }
+
+  test("SPARK-58096: approx_top_k_accumulate/estimate keeps 
ICU-collation-distinct " +
+    "non-ASCII values separate through the serde round-trip") {
+    val res = sql(
+      s"""SELECT approx_top_k_estimate(approx_top_k_accumulate(c), 3)

Review Comment:
   ```suggestion
         """SELECT approx_top_k_estimate(approx_top_k_accumulate(c), 3)
   ```



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