Pastoray opened a new pull request, #27273: URL: https://github.com/apache/datafusion/pull/27273
> [!IMPORTANT] > > **Fork CI setup:** For pull requests from a fork to `main`, enable workflows in your fork's **Actions** tab before you push. > See the [fork CI setup guide](https://datafusion.apache.org/contributor-guide/#pull-requests-from-forks). ## Which issue does this PR close? - Closes #27186. ## Rationale for this change The Spark-compatible `encode` function in `datafusion-spark` follows Spark 3.5, but Spark 4.0 and later behave differently in two ways: - **Charset names:** Spark 4 accepts only `us-ascii`, `iso-8859-1`, `utf-8`, `utf-16be`, `utf-16le`, `utf-16` and `utf-32` (case-insensitive). Any other name, such as `utf-32be`, `UTF8` or `latin1`, raises `[INVALID_PARAMETER_VALUE.CHARSET]`. DataFusion accepted these. - **Unmappable characters:** Spark 4 raises `[MALFORMED_CHARACTER_CODING]` when the charset cannot represent a character, for example `encode('é', 'us-ascii')`. DataFusion silently replaced the character with `?`. Users who rely on DataFusion to reproduce Spark 4 results therefore got a value where Spark returns an error. ## What changes are included in this PR? - Add two options under `datafusion.spark`, mirroring Spark's legacy configs and defaulting to `false` as in Spark: - `legacy_java_charsets` (`spark.sql.legacy.javaCharsets`): also accept the Spark 3.x charset aliases (`UTF8`, `LATIN1`, ...) and `UTF-32BE` / `UTF-32LE`. - `legacy_coding_error_action` (`spark.sql.legacy.codingErrorAction`): replace unmappable characters with `?` instead of raising an error. - `encode` now resolves the charset into an enum, validates it against the Spark 4 allowlist, and handles unmappable characters according to the two options. Charset resolution happens per row, so a bad charset in a column is reported only for rows that are actually encoded, and NULL values or charsets still yield NULL. - Regenerate `docs/source/user-guide/configs.md` and add a note to the 56.0.0 upgrade guide. Setting both options to `true` restores the previous behavior. ## What is the testing strategy for this PR? - `datafusion/sqllogictest/test_files/spark/string/encode.slt` is rewritten to cover the Spark 4 defaults (accepted charsets in any case, rejected names, unmappable characters in `us-ascii` / `iso-8859-1`, per-row charsets, NULL handling, binary input with invalid UTF-8) and, in separate sections, each legacy option on its own and both together. - `information_schema.slt` is updated for the two new options. - Unit tests in `encode.rs` cover the same cases at function level, including that the two options are independent of each other. ## Are there any user-facing changes? Yes. The default behavior of the Spark `encode` function changes to match Spark 4, so queries using other charset names or text the charset cannot represent now error instead of succeeding. This is documented in the 56.0.0 upgrade guide, and setting `datafusion.spark.legacy_java_charsets` and `datafusion.spark.legacy_coding_error_action` to `true` restores the old behavior. There are no Rust public API changes, but there are two new config options. ## Notes for reviewers - The defaults follow Spark 4. I'm happy to flip them if you would rather keep Spark 3.5 as the default. - The error messages are modeled on Spark's, but I have not compared them against a real Spark 4 run. - Even with `legacy_java_charsets` on, only the charsets `encode` already supported are accepted, not every Java charset name as in Spark 3.x. -- 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]
