feat(spark): add to_binary and try_to_binary - #24266
Conversation
Implements `to_binary(str[, fmt])` and `try_to_binary(str[, fmt])` for the datafusion-spark crate. `fmt` is a case-insensitive literal of `hex`, `utf-8`, `utf8` or `base64`, defaulting to `hex`. The conversion is done in the kernel rather than by delegating to existing functions: Spark rewrites `to_binary` onto `Unhex`/`Encode`/`UnBase64` with `failOnError = true`, but the equivalents here disagree on which inputs error and which return NULL, so a single `fail_on_error` flag drives both variants. `unbase64` also has no kernel to call -- it exists only as a `simplify()` rewrite -- and core's base64 decoder is private. Hex decoding reuses the existing `unhex_scalar`, which is made `pub(crate)` rather than duplicated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
cc @Jefffrey This is my first time contribution. Can you please take a look? |
Jefffrey
left a comment
There was a problem hiding this comment.
ci errors are because base64 has a 0.23.1 now, so need to update the lockfile (just rerun cargo check/build and commit again)
| impl SparkToBinary { | ||
| pub fn new() -> Self { | ||
| Self { | ||
| signature: to_binary_signature(), |
There was a problem hiding this comment.
| signature: to_binary_signature(), | |
| signature: Signature::one_of( | |
| vec![TypeSignature::String(1), TypeSignature::String(2)], | |
| Volatility::Immutable, | |
| ), |
same for trytobinary
| Some(bytes) => Ok(Some(bytes)), | ||
| None if fail_on_error => exec_err!( | ||
| "{name}: cannot convert '{value}' to binary using format '{}'", | ||
| match format { |
There was a problem hiding this comment.
this match should instead be inside a Display impl on BinaryFormat
| fail_on_error: bool, | ||
| ) -> Result<ArrayRef> { | ||
| let values: Vec<Option<&str>> = match array.data_type() { | ||
| DataType::Utf8 => array.as_string::<i32>().iter().collect(), |
There was a problem hiding this comment.
we can probably avoid these collects, e.g. could have a method with the main loop taking an iter and we pass in the iter
| #[test] | ||
| fn test_null_inputs() -> Result<()> { | ||
| // a NULL value yields NULL | ||
| let args = vec![ColumnarValue::Scalar(ScalarValue::Utf8(None))]; |
There was a problem hiding this comment.
can some of these tests which rely on invoking the scalar udf be moved to SLTs instead
| statement error | ||
| SELECT to_binary('zz'::string, 'hex'::string); | ||
|
|
||
| statement error | ||
| SELECT to_binary('a!'::string, 'base64'::string); | ||
|
|
||
| # ...and on a format it does not recognise | ||
| statement error |
There was a problem hiding this comment.
can we assert at least some of the error message
Which issue does this PR close?
datafusion-sparkSpark Compatible Functions #15914Rationale for this change
Spark's
to_binary(str[, fmt])converts a string to binary, choosing how basedon
fmt:hex,utf-8/utf8, orbase64.try_to_binaryis the samefunction except that input it cannot convert yields NULL instead of raising an
error.
Neither is available in the
datafusion-sparkcrate, and no core DataFusionfunction can stand in for them:
decodeaccepts onlybase64,base64padandhex, so there is noway to ask it for Spark's
utf-8conversiondecodealways raises on input it cannot convert, so there is nothingto build
try_to_binary's NULL-returning behaviour fromWhat changes are included in this PR?
SparkToBinaryandSparkTryToBinaryUDFs indatafusion/spark/src/function/string/to_binary.rsto_binary(str[, fmt])wherefmtis a case-insensitive literal ofhex,utf-8,utf8orbase64, defaulting tohexhexdecodes two characters per byte, left-padding an odd-length inputwith
0, matchingUnhexutf-8/utf8returns the string's own UTF-8 bytesbase64matches Java's MIME decoder, which Spark uses: the standardalphabet, padding optional, and the unused trailing bits of a short final
group ignored
fmtyields NULL in both functionsto_binaryraises on a malformed value or an unrecognisedfmt;try_to_binaryreturns NULL for both, matchingnullOnInvalidFormatfmtmust be foldable, as Spark requiresunhex_scalaris madepub(crate)and reused for thehexpath ratherthan duplicating the decoder
mod.rs(make_udf_function!,export_functions!,functions())Are these changes tested?
Yes.
to_binary.rs(each format, hex as the default, emptyinput, NULL value and NULL format, invalid value, invalid format, column
input, and a column where one row is invalid)
spark/string/to_binary.sltandspark/string/try_to_binary.sltAre there any user-facing changes?
No. These are new functions in the
datafusion-sparkcrate only.