[SPARK-58096][SQL] Fix approx_top_k wrong counts/items for UNICODE-collated non-ASCII strings

[jiwen624]

What changes were proposed in this pull request?

Follow-up to #57177.
approx_top_k on a non-binary collated string keyed its DataSketches item on getCollationKey(...).toString(). ICU sort keys are arbitrary bytes, not valid UTF-8, so .toString() lossily maps them to U+FFFD and merges collation-distinct values into one item with an inflated count. This changes CollatedString to key on the raw sort-key byte[] (via CollationFactory.getCollationKeyBytes), so key and original round-trip losslessly with no Java String intermediary. The on-wire serde format is unchanged (only original is persisted).

Why are the changes needed?

Wrong counts for any UNICODE/UNICODE_CI/locale collation over non-ASCII data (e.g. 且/丕/世 collapse to one item). UTF8_BINARY and UTF8_LCASE are unaffected.

SELECT approx_top_k(c, 10)
FROM (SELECT CAST(col AS STRING COLLATE UNICODE_CI) AS c
      FROM VALUES ('且'),('且'),('且'),('丕'),('丕'),('世') AS t(col));
-- returns a SINGLE item with count 6, instead of 3 items with counts 3, 2, 1

Does this PR introduce any user-facing change?

Yes — fixes incorrect approx_top_k counts/items for ICU-collated non-ASCII strings.

How was this patch tested?

Added UT case.

Was this patch authored or co-authored using generative AI tooling?

Yes

[jiwen624]

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

[jiwen624]

Hi @cloud-fan this patch fixes the approx_top_k wrong count/result issue which is the follow-up to #57177 Thank you.

[cloud-fan]

0 blocking, 1 non-blocking, 0 nits.
Clean, well-scoped fix — keying on raw sort-key bytes matches the Mode analogue and correctly avoids the lossy-toString over-merge at both key-producing sites. One optional test-coverage note.

Suggestions (1)

• ApproxTopKSuite.scala:314: non-ASCII regression coverage is missing for the accumulate/estimate/combine (serde deserialize) path — see inline

[cloud-fan]

The same lossy-toString bug also lived in ArrayOfCollatedStringsSerDe.wrap (the deserialize path), and the fix corrects it there too — but this new test only exercises the direct approx_top_k update path. The existing SPARK-58069 accumulate/combine tests above use ASCII-only fixtures ('HELLO'/'world'), which can't observe the over-merge, so the serde path currently has no non-ASCII regression guard. Consider mirroring this case through approx_top_k_estimate(approx_top_k_accumulate(...)) (and/or combine) to cover that second key-producing site. Non-blocking.

[jiwen624]

Thanks. Added test coverage 👍