MaxGekk commented on code in PR #56114: URL: https://github.com/apache/spark/pull/56114#discussion_r3452595611
########## docs/sql-migration-guide.md: ########## @@ -80,6 +80,7 @@ license: | - Since Spark 4.0, The Storage-Partitioned Join feature flag `spark.sql.sources.v2.bucketing.pushPartValues.enabled` is set to `true`. To restore the previous behavior, set `spark.sql.sources.v2.bucketing.pushPartValues.enabled` to `false`. - Since Spark 4.0, the `sentences` function uses `Locale(language)` instead of `Locale.US` when `language` parameter is not `NULL` and `country` parameter is `NULL`. - Since Spark 4.0, reading from a file source table will correctly respect query options, e.g. delimiters. Previously, the first query plan was cached and subsequent option changes ignored. To restore the previous behavior, set `spark.sql.legacy.readFileSourceTableCacheIgnoreOptions` to `true`. +- Since Spark 4.0, string columns may carry a non-binary collation (`UTF8_LCASE`, `UTF8_LCASE_TRIM`, or any ICU collation). `DataFrameStatFunctions.bloomFilter` builds the underlying sketch using raw UTF-8 byte equality and is therefore collation-blind: two values that compare equal under the column's collation will occupy distinct slots and `mightContain` may return inconsistent results. For collation-consistent membership, build the filter over a column declared with the default `UTF8_BINARY` collation, or normalize the values yourself before building and querying. For ASCII data under `UTF8_LCASE`, `lower(col)` is sufficient; for `UTF8_LCASE_TRIM`, use `lower(trim(col))`. Note that `lower()` does not correctly handle certain non-ASCII Unicode cases (for example Turkish dotted/dotless 'I'/'i' or German 'ß'/'SS'); see [SPARK-57055](https://issues.apache.org/jira/browse/SPARK-57055) for a discussion of adding a public canonical-form helper for ICU collations. Review Comment: Two related factual errors in the trim-collation guidance here: 1. **Collation name.** `UTF8_LCASE_TRIM` is not a valid collation. The actual name is `UTF8_LCASE_RTRIM` (`CollationFactory` defines an `RTRIM` / right-trim modifier, not a both-sides `TRIM`). Both occurrences in this bullet should be `UTF8_LCASE_RTRIM`. 2. **Workaround.** `lower(trim(col))` is wrong for an RTRIM collation. RTRIM ignores only *trailing* spaces, so the normalization that mirrors the collation is `lower(rtrim(col))`. `trim()` is both-sides (`StringTrim`) while `rtrim()` is trailing-only (`StringTrimRight`), so `lower(trim(col))` also strips *leading* spaces -- collapsing values the collation treats as distinct (e.g. `" a"` vs `"a"`) into the same bloom slot. That makes `mightContain` *more* lenient than the collation, reintroducing the inconsistency this bullet is warning about. Suggested: `UTF8_LCASE_TRIM` -> `UTF8_LCASE_RTRIM`, and `lower(trim(col))` -> `lower(rtrim(col))`. (The `@note` Scaladoc doesn't mention trim collations, so it's unaffected.) -- 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]
