andygrove commented on issue #12047: URL: https://github.com/apache/arrow-rs/issues/12047#issuecomment-6618965660
+1 to this. DataFusion Comet would use `Replace`, and it would let us close several issues tracked in apache/datafusion-comet#4764. Two points on the design. **The check should key on the target type, not only the annotation.** The byte array decoders validate UTF-8 only when the column is annotated `UTF8`. A BINARY column without the annotation that is read as `Utf8`, `LargeUtf8`, `Utf8View`, or a dictionary of those, through `ArrowReaderOptions::with_schema`, is never checked, so it produces an invalid string array today (#12063). With the check keyed on the annotation alone, `Replace` would not apply there, and that is the other half of what Comet needs: Spark reads such columns as strings with `spark.sql.parquet.binaryAsString`. DataFusion also hits this through its binary-to-string schema coercion (apache/datafusion#27065). I had opened #12064 to fix that case on its own (validate whenever the target is a string type, and error on invalid UTF-8). I have closed it in favor of folding it into this option, since `Error`/`Replace` is the better place to decide what happens. The branch may still be useful: it threads the flag through the byte array, byte view and dictionary decoders, fixes the dictionary decoder's value type for string dictionaries over unannotated columns (they returned `Binary` values, or panicked for `Utf8View`), and has a test over the six string types with plain and dictionary encoding. **Replacement semantics.** `String::from_utf8_lossy` follows the Unicode "maximal subpart" practice. The JVM's UTF-8 decoder, which Spark uses, emits a different number of U+FFFD for some ill-formed sequences, encoded surrogates (`ED A0..BF ..`) among them. For Comet it would help if `Replace` matched the JVM, or if the replacement were pluggable. If that is out of scope, matching `from_utf8_lossy` is still a big improvement, and we can document the difference. Are you planning to implement this? If not, I'd be happy to put up a PR building on the #12064 branch. -- 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]
