From e92b63ca6589642e31ffd9bc59d86e876475c251 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 21 Jul 2026 11:51:07 -0600 Subject: [PATCH 1/9] feat(common): add shared hex encoding utilities --- datafusion/common/src/utils/hex.rs | 243 +++++++++++++++++++++++++++++ datafusion/common/src/utils/mod.rs | 1 + 2 files changed, 244 insertions(+) create mode 100644 datafusion/common/src/utils/hex.rs diff --git a/datafusion/common/src/utils/hex.rs b/datafusion/common/src/utils/hex.rs new file mode 100644 index 0000000000000..582ae5f35f1c1 --- /dev/null +++ b/datafusion/common/src/utils/hex.rs @@ -0,0 +1,243 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Hex encoding shared across the workspace. +//! +//! `to_hex`, Spark's `hex`, and the digest functions (`md5`, `sha1`, `sha2`) +//! all need the same conversion. Keeping one implementation here avoids the +//! per-crate lookup tables that previously diverged in both speed and case +//! handling. + +/// Case of the emitted hex digits. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum HexCase { + /// Digits `0123456789abcdef`. + Lower, + /// Digits `0123456789ABCDEF`. + Upper, +} + +const LOWER_DIGITS: &[u8; 16] = b"0123456789abcdef"; +const UPPER_DIGITS: &[u8; 16] = b"0123456789ABCDEF"; + +/// Maps a full byte to its two hex digits, so encoding advances a whole byte +/// per iteration instead of a nibble. +const LOOKUP_LOWER: [[u8; 2]; 256] = build_lookup(LOWER_DIGITS); +const LOOKUP_UPPER: [[u8; 2]; 256] = build_lookup(UPPER_DIGITS); + +const fn build_lookup(digits: &[u8; 16]) -> [[u8; 2]; 256] { + let mut table = [[0u8; 2]; 256]; + let mut i = 0; + while i < 256 { + table[i][0] = digits[(i >> 4) & 0xF]; + table[i][1] = digits[i & 0xF]; + i += 1; + } + table +} + +impl HexCase { + #[inline] + const fn lookup(self) -> &'static [[u8; 2]; 256] { + match self { + HexCase::Lower => &LOOKUP_LOWER, + HexCase::Upper => &LOOKUP_UPPER, + } + } + + #[inline] + const fn digits(self) -> &'static [u8; 16] { + match self { + HexCase::Lower => LOWER_DIGITS, + HexCase::Upper => UPPER_DIGITS, + } + } +} + +/// Appends the hex encoding of `bytes` to `out`. +/// +/// Allocates only through `out`'s own growth. Callers that must bound or guard +/// that growth should reserve capacity in `out` before calling. +#[inline] +pub fn encode_bytes_into(bytes: &[u8], case: HexCase, out: &mut Vec) { + let lookup = case.lookup(); + for &byte in bytes { + out.extend_from_slice(&lookup[byte as usize]); + } +} + +/// Returns the hex encoding of `bytes` as an owned `String`. +#[inline] +pub fn encode_bytes(bytes: &[u8], case: HexCase) -> String { + let mut out = Vec::with_capacity(bytes.len() * 2); + encode_bytes_into(bytes, case, &mut out); + // SAFETY: `out` holds only ASCII hex digits, which are valid UTF-8. + unsafe { String::from_utf8_unchecked(out) } +} + +/// Writes `v` as hex into `buf` and returns the written subslice. +/// +/// Digits are written right-aligned with leading zeros trimmed, so the result +/// borrows the tail of `buf`. Zero encodes as `"0"`. +/// +/// Signed values should be cast with `as u64`, which yields the two's +/// complement representation that both `to_hex` and Spark's `hex` produce for +/// negative input. +#[inline] +pub fn encode_u64(v: u64, case: HexCase, buf: &mut [u8; 16]) -> &[u8] { + let start = write_digits(v, case, buf); + &buf[start..] +} + +/// Writes the digits of `v` right-aligned in `buf`, returning the index of the +/// first digit. +/// +/// Split out from [`encode_u64`] so the mutable borrow of `buf` ends before the +/// returned slice reborrows it; a conditional `return &buf[..]` inside the loop +/// body would outlive the writes that follow it. +#[inline] +fn write_digits(v: u64, case: HexCase, buf: &mut [u8; 16]) -> usize { + if v == 0 { + buf[15] = b'0'; + return 15; + } + + // Consume two nibbles (one full byte) per iteration. + let lookup = case.lookup(); + let mut pos = 16; + let mut rest = v; + while rest >= 0x10 { + pos -= 2; + let pair = lookup[(rest & 0xFF) as usize]; + buf[pos] = pair[0]; + buf[pos + 1] = pair[1]; + rest >>= 8; + } + if rest > 0 { + // A single high nibble (0x1..=0xF) remains. + pos -= 1; + buf[pos] = case.digits()[rest as usize]; + } + + pos +} + +#[cfg(test)] +mod tests { + use super::*; + + fn hex_u64(v: u64, case: HexCase) -> String { + let mut buf = [0u8; 16]; + String::from_utf8(encode_u64(v, case, &mut buf).to_vec()).unwrap() + } + + #[test] + fn encode_u64_zero() { + assert_eq!(hex_u64(0, HexCase::Lower), "0"); + assert_eq!(hex_u64(0, HexCase::Upper), "0"); + } + + #[test] + fn encode_u64_single_nibble() { + for v in 1..=0xFu64 { + assert_eq!(hex_u64(v, HexCase::Lower), format!("{v:x}")); + assert_eq!(hex_u64(v, HexCase::Upper), format!("{v:X}")); + } + } + + #[test] + fn encode_u64_digit_count_boundaries() { + // Straddle each odd/even digit-count boundary: the two-nibbles-per + // iteration loop plus the trailing single-nibble fixup. + for v in [ + 0x10u64, + 0xFF, + 0x100, + 0xFFF, + 0x1000, + 0xFFFFF, + 0xFFFF_FFFF, + 0x1_0000_0000, + ] { + assert_eq!(hex_u64(v, HexCase::Lower), format!("{v:x}")); + assert_eq!(hex_u64(v, HexCase::Upper), format!("{v:X}")); + } + } + + #[test] + fn encode_u64_max() { + assert_eq!(hex_u64(u64::MAX, HexCase::Lower), "ffffffffffffffff"); + assert_eq!(hex_u64(u64::MAX, HexCase::Upper), "FFFFFFFFFFFFFFFF"); + } + + #[test] + fn encode_u64_signed_is_twos_complement() { + // Callers cast signed values with `as u64`; this is the behaviour both + // `to_hex` and Spark `hex` rely on for negative input. + assert_eq!(hex_u64(-1i64 as u64, HexCase::Lower), "ffffffffffffffff"); + assert_eq!(hex_u64(i64::MIN as u64, HexCase::Upper), "8000000000000000"); + } + + #[test] + fn encode_bytes_empty() { + assert_eq!(encode_bytes(&[], HexCase::Lower), ""); + assert_eq!(encode_bytes(&[], HexCase::Upper), ""); + } + + #[test] + fn encode_bytes_examples() { + assert_eq!(encode_bytes(&[0x00], HexCase::Lower), "00"); + assert_eq!(encode_bytes(&[0xAB], HexCase::Lower), "ab"); + assert_eq!(encode_bytes(&[0xAB], HexCase::Upper), "AB"); + assert_eq!( + encode_bytes(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower), + "deadbeef" + ); + assert_eq!( + encode_bytes(&[0xde, 0xad, 0xbe, 0xef], HexCase::Upper), + "DEADBEEF" + ); + } + + #[test] + fn encode_bytes_covers_every_byte_value() { + let bytes: Vec = (0..=255u8).collect(); + + let expected: String = bytes.iter().map(|b| format!("{b:02x}")).collect(); + assert_eq!(encode_bytes(&bytes, HexCase::Lower), expected); + + let expected: String = bytes.iter().map(|b| format!("{b:02X}")).collect(); + assert_eq!(encode_bytes(&bytes, HexCase::Upper), expected); + } + + #[test] + fn encode_bytes_into_appends_without_clearing() { + let mut out = b"prefix-".to_vec(); + encode_bytes_into(&[0x01, 0x02], HexCase::Lower, &mut out); + assert_eq!(out, b"prefix-0102"); + } + + #[test] + fn encode_bytes_agrees_with_encode_bytes_into() { + let bytes: Vec = (0..=255u8).collect(); + for case in [HexCase::Lower, HexCase::Upper] { + let mut out = Vec::new(); + encode_bytes_into(&bytes, case, &mut out); + assert_eq!(String::from_utf8(out).unwrap(), encode_bytes(&bytes, case)); + } + } +} diff --git a/datafusion/common/src/utils/mod.rs b/datafusion/common/src/utils/mod.rs index 94bbb91a7fa8b..73772b319351c 100644 --- a/datafusion/common/src/utils/mod.rs +++ b/datafusion/common/src/utils/mod.rs @@ -19,6 +19,7 @@ pub(crate) mod aggregate; pub mod expr; +pub mod hex; pub mod memory; pub mod proxy; pub mod string_utils; From 997187c2631cb0b1f134253f1908d37b2feee120 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 21 Jul 2026 12:13:05 -0600 Subject: [PATCH 2/9] refactor(functions): use shared hex utilities in to_hex --- datafusion/functions/src/string/to_hex.rs | 126 +++++++--------------- 1 file changed, 36 insertions(+), 90 deletions(-) diff --git a/datafusion/functions/src/string/to_hex.rs b/datafusion/functions/src/string/to_hex.rs index 497a0a1206922..a6bcd179664df 100644 --- a/datafusion/functions/src/string/to_hex.rs +++ b/datafusion/functions/src/string/to_hex.rs @@ -24,6 +24,7 @@ use arrow::datatypes::{ Int64Type, UInt8Type, UInt16Type, UInt32Type, UInt64Type, }; use datafusion_common::cast::as_primitive_array; +use datafusion_common::utils::hex::{HexCase, encode_u64}; use datafusion_common::{Result, ScalarValue, exec_err, internal_err}; use datafusion_expr::{ Coercion, ColumnarValue, Documentation, ScalarFunctionArgs, ScalarUDFImpl, Signature, @@ -31,9 +32,6 @@ use datafusion_expr::{ }; use datafusion_macros::user_doc; -/// Hex lookup table for fast conversion -const HEX_CHARS: &[u8; 16] = b"0123456789abcdef"; - /// Converts the number to its equivalent hexadecimal representation. /// to_hex(2147483647) = '7fffffff' fn to_hex_array(array: &ArrayRef) -> Result @@ -59,8 +57,7 @@ where // Process all values directly (including null slots - we write empty strings for nulls) // The null bitmap will mark which entries are actually null for value in integer_array.values() { - let hex_len = value.write_hex_to_buffer(&mut hex_buffer); - values.extend_from_slice(&hex_buffer[16 - hex_len..]); + values.extend_from_slice(value.write_hex(&mut hex_buffer)); offsets.push(values.len() as i32); } @@ -79,101 +76,50 @@ where #[inline] fn to_hex_scalar(value: T) -> String { let mut hex_buffer = [0u8; 16]; - let hex_len = value.write_hex_to_buffer(&mut hex_buffer); - // SAFETY: hex_buffer is ASCII hex digits - unsafe { std::str::from_utf8_unchecked(&hex_buffer[16 - hex_len..]).to_string() } + let hex = value.write_hex(&mut hex_buffer); + // SAFETY: hex holds only ASCII hex digits. + unsafe { std::str::from_utf8_unchecked(hex).to_string() } } /// Trait for converting integer types to hexadecimal in a buffer trait ToHex: ArrowNativeType { - /// Write hex representation to buffer and return the number of hex digits written. - /// The hex digits are written right-aligned in the buffer (starting from position 16 - len). - fn write_hex_to_buffer(self, buffer: &mut [u8; 16]) -> usize; -} - -/// Write unsigned value to hex buffer and return the number of digits written. -/// Digits are written right-aligned in the buffer. -#[inline] -fn write_unsigned_hex_to_buffer(value: u64, buffer: &mut [u8; 16]) -> usize { - if value == 0 { - buffer[15] = b'0'; - return 1; - } - - // Write hex digits from right to left - let mut pos = 16; - let mut v = value; - while v > 0 { - pos -= 1; - buffer[pos] = HEX_CHARS[(v & 0xf) as usize]; - v >>= 4; - } - - 16 - pos -} - -/// Write signed value to hex buffer (two's complement for negative) and return digit count -#[inline] -fn write_signed_hex_to_buffer(value: i64, buffer: &mut [u8; 16]) -> usize { - // For negative values, use two's complement representation (same as casting to u64) - write_unsigned_hex_to_buffer(value as u64, buffer) -} - -impl ToHex for i8 { - #[inline] - fn write_hex_to_buffer(self, buffer: &mut [u8; 16]) -> usize { - write_signed_hex_to_buffer(self as i64, buffer) - } -} - -impl ToHex for i16 { - #[inline] - fn write_hex_to_buffer(self, buffer: &mut [u8; 16]) -> usize { - write_signed_hex_to_buffer(self as i64, buffer) - } -} - -impl ToHex for i32 { - #[inline] - fn write_hex_to_buffer(self, buffer: &mut [u8; 16]) -> usize { - write_signed_hex_to_buffer(self as i64, buffer) - } -} - -impl ToHex for i64 { - #[inline] - fn write_hex_to_buffer(self, buffer: &mut [u8; 16]) -> usize { - write_signed_hex_to_buffer(self, buffer) - } + /// Writes the hex representation into `buf` and returns the written + /// subslice. Digits are right-aligned in `buf` with leading zeros trimmed. + fn write_hex(self, buf: &mut [u8; 16]) -> &[u8]; } -impl ToHex for u8 { - #[inline] - fn write_hex_to_buffer(self, buffer: &mut [u8; 16]) -> usize { - write_unsigned_hex_to_buffer(self as u64, buffer) - } -} - -impl ToHex for u16 { - #[inline] - fn write_hex_to_buffer(self, buffer: &mut [u8; 16]) -> usize { - write_unsigned_hex_to_buffer(self as u64, buffer) - } +/// Signed values use their two's complement representation, matching a cast to +/// the corresponding unsigned type. +macro_rules! impl_to_hex_signed { + ($ty:ty) => { + impl ToHex for $ty { + #[inline] + fn write_hex(self, buf: &mut [u8; 16]) -> &[u8] { + encode_u64(self as i64 as u64, HexCase::Lower, buf) + } + } + }; } -impl ToHex for u32 { - #[inline] - fn write_hex_to_buffer(self, buffer: &mut [u8; 16]) -> usize { - write_unsigned_hex_to_buffer(self as u64, buffer) - } +macro_rules! impl_to_hex_unsigned { + ($ty:ty) => { + impl ToHex for $ty { + #[inline] + fn write_hex(self, buf: &mut [u8; 16]) -> &[u8] { + encode_u64(self as u64, HexCase::Lower, buf) + } + } + }; } -impl ToHex for u64 { - #[inline] - fn write_hex_to_buffer(self, buffer: &mut [u8; 16]) -> usize { - write_unsigned_hex_to_buffer(self, buffer) - } -} +impl_to_hex_signed!(i8); +impl_to_hex_signed!(i16); +impl_to_hex_signed!(i32); +impl_to_hex_signed!(i64); +impl_to_hex_unsigned!(u8); +impl_to_hex_unsigned!(u16); +impl_to_hex_unsigned!(u32); +impl_to_hex_unsigned!(u64); #[user_doc( doc_section(label = "String Functions"), From 33d015e613ce6b38db1d5864c294b8e47262ce97 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 21 Jul 2026 12:24:08 -0600 Subject: [PATCH 3/9] refactor(spark): use shared hex utilities in hex Migrate the Spark-compatible hex function onto the shared datafusion_common::utils::hex module (encode_bytes_into, encode_u64) instead of its own lookup tables, keeping the post-#23473 buffer structure (manual value/offset buffers, try_reserve guard, StringArray::new_unchecked) unchanged. --- datafusion/spark/src/function/math/hex.rs | 100 +++------------------- 1 file changed, 13 insertions(+), 87 deletions(-) diff --git a/datafusion/spark/src/function/math/hex.rs b/datafusion/spark/src/function/math/hex.rs index a283bd8fa7de6..c9814a94e82dd 100644 --- a/datafusion/spark/src/function/math/hex.rs +++ b/datafusion/spark/src/function/math/hex.rs @@ -28,6 +28,7 @@ use arrow::{ use datafusion_common::cast::as_large_binary_array; use datafusion_common::cast::as_string_view_array; use datafusion_common::types::{NativeType, logical_int64, logical_string}; +use datafusion_common::utils::hex::{HexCase, encode_bytes_into, encode_u64}; use datafusion_common::utils::take_function_args; use datafusion_common::{ DataFusionError, @@ -110,54 +111,6 @@ impl ScalarUDFImpl for SparkHex { } } -/// Hex encoding lookup tables for fast byte-to-hex conversion. -/// -/// Each entry maps a full byte to its two-character hex encoding so the -/// hot loop becomes one load + one two-byte extend per input byte instead -/// of two nibble lookups and two pushes. -const HEX_CHARS_UPPER_NIBBLES: &[u8; 16] = b"0123456789ABCDEF"; -const HEX_CHARS_LOWER_NIBBLES: &[u8; 16] = b"0123456789abcdef"; - -const HEX_LOOKUP_UPPER: [[u8; 2]; 256] = build_hex_lookup(HEX_CHARS_UPPER_NIBBLES); -const HEX_LOOKUP_LOWER: [[u8; 2]; 256] = build_hex_lookup(HEX_CHARS_LOWER_NIBBLES); - -const fn build_hex_lookup(nibbles: &[u8; 16]) -> [[u8; 2]; 256] { - let mut table = [[0u8; 2]; 256]; - let mut i = 0; - while i < 256 { - table[i][0] = nibbles[(i >> 4) & 0xF]; - table[i][1] = nibbles[i & 0xF]; - i += 1; - } - table -} - -#[inline] -fn hex_int64(num: i64, buffer: &mut [u8; 16]) -> &[u8] { - if num == 0 { - return b"0"; - } - - // Walk the value two nibbles (one full byte) at a time. The buffer is - // filled from the right so the high-order nibbles end up first; the - // returned slice trims leading zeros automatically. - let mut n = num as u64; - let mut i = 16; - while n >= 0x10 { - i -= 2; - let pair = HEX_LOOKUP_UPPER[(n & 0xFF) as usize]; - buffer[i] = pair[0]; - buffer[i + 1] = pair[1]; - n >>= 8; - } - if n > 0 { - // Single remaining high nibble (value 0x1..=0xF). - i -= 1; - buffer[i] = HEX_CHARS_UPPER_NIBBLES[n as usize]; - } - &buffer[i..] -} - /// Generic hex encoding for byte array types fn hex_encode_bytes<'a, I, T>( iter: I, @@ -168,10 +121,10 @@ where I: Iterator>, T: AsRef<[u8]> + 'a, { - let lookup = if lowercase { - &HEX_LOOKUP_LOWER + let case = if lowercase { + HexCase::Lower } else { - &HEX_LOOKUP_UPPER + HexCase::Upper }; // Write hex digits directly into one growing value buffer, tracking offsets @@ -195,9 +148,7 @@ where "failed to reserve {additional} bytes for hex output: {e}" ) })?; - for &byte in bytes { - values.extend_from_slice(&lookup[byte as usize]); - } + encode_bytes_into(bytes, case, &mut values); nulls.append_non_null(); } else { nulls.append_null(); @@ -233,7 +184,7 @@ fn hex_encode_int64( for v in iter { if let Some(num) = v { let mut temp = [0u8; 16]; - let slice = hex_int64(num, &mut temp); + let slice = encode_u64(num as u64, HexCase::Upper, &mut temp); // SAFETY: slice contains only ASCII hex digests, which are valid UTF-8 unsafe { builder.append_value(from_utf8_unchecked(slice)); @@ -381,7 +332,6 @@ pub fn compute_hex( #[cfg(test)] mod test { - use std::str::from_utf8_unchecked; use std::sync::Arc; use arrow::array::{ @@ -486,7 +436,7 @@ mod test { #[test] fn test_hex_int64() { - let test_cases = vec![ + let cases = vec![ (0_i64, "0"), (1, "1"), (15, "F"), @@ -499,36 +449,12 @@ mod test { (-1, "FFFFFFFFFFFFFFFF"), ]; - for (num, expected) in test_cases { - let mut cache = [0u8; 16]; - let slice = super::hex_int64(num, &mut cache); - - unsafe { - let result = from_utf8_unchecked(slice); - assert_eq!(expected, result, "hex_int64({num}) mismatch"); - } - } - } - - #[test] - fn test_hex_lookup_table_covers_all_bytes() { - // Cross-check the precomputed table against an independent encoder - // for every possible byte value and both casings. - for byte in 0u8..=255 { - let upper = format!("{byte:02X}"); - let lower = format!("{byte:02x}"); - let upper_pair = super::HEX_LOOKUP_UPPER[byte as usize]; - let lower_pair = super::HEX_LOOKUP_LOWER[byte as usize]; - assert_eq!( - upper.as_bytes(), - &upper_pair, - "upper encoding mismatch for byte 0x{byte:02X}" - ); - assert_eq!( - lower.as_bytes(), - &lower_pair, - "lower encoding mismatch for byte 0x{byte:02X}" - ); + let arr = + super::hex_encode_int64(cases.iter().map(|(n, _)| Some(*n)), cases.len()) + .unwrap(); + let arr = as_string_array(&arr); + for (i, (num, expected)) in cases.iter().enumerate() { + assert_eq!(*expected, arr.value(i), "hex({num})"); } } From 152fef0b2c5e9e0ca6b56ef7e4c683aa2e3c0951 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 21 Jul 2026 12:29:56 -0600 Subject: [PATCH 4/9] refactor(functions): use shared hex utilities in md5 --- datafusion/functions/src/crypto/md5.rs | 29 +++++++------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/datafusion/functions/src/crypto/md5.rs b/datafusion/functions/src/crypto/md5.rs index 178aebf0fbd41..b1206d2e423cc 100644 --- a/datafusion/functions/src/crypto/md5.rs +++ b/datafusion/functions/src/crypto/md5.rs @@ -21,6 +21,7 @@ use datafusion_common::{ cast::as_binary_array, internal_err, types::{logical_binary, logical_string}, + utils::hex::{HexCase, encode_bytes}, utils::take_function_args, }; use datafusion_expr::{ @@ -98,22 +99,6 @@ impl ScalarUDFImpl for Md5Func { } } -/// Hex encoding lookup table for fast byte-to-hex conversion -const HEX_CHARS_LOWER: &[u8; 16] = b"0123456789abcdef"; - -/// Fast hex encoding using a lookup table instead of format strings. -/// This is significantly faster than using `write!("{:02x}")` for each byte. -#[inline] -fn hex_encode(data: impl AsRef<[u8]>) -> String { - let bytes = data.as_ref(); - let mut s = String::with_capacity(bytes.len() * 2); - for &b in bytes { - s.push(HEX_CHARS_LOWER[(b >> 4) as usize] as char); - s.push(HEX_CHARS_LOWER[(b & 0x0f) as usize] as char); - } - s -} - fn md5(args: &[ColumnarValue]) -> Result { let [data] = take_function_args("md5", args)?; let value = digest_process(data, DigestAlgorithm::Md5)?; @@ -122,13 +107,15 @@ fn md5(args: &[ColumnarValue]) -> Result { Ok(match value { ColumnarValue::Array(array) => { let binary_array = as_binary_array(&array)?; - let string_array: StringViewArray = - binary_array.iter().map(|opt| opt.map(hex_encode)).collect(); + let string_array: StringViewArray = binary_array + .iter() + .map(|opt| opt.map(|b| encode_bytes(b, HexCase::Lower))) + .collect(); ColumnarValue::Array(Arc::new(string_array)) } - ColumnarValue::Scalar(ScalarValue::Binary(opt)) => { - ColumnarValue::Scalar(ScalarValue::Utf8View(opt.map(hex_encode))) - } + ColumnarValue::Scalar(ScalarValue::Binary(opt)) => ColumnarValue::Scalar( + ScalarValue::Utf8View(opt.map(|b| encode_bytes(&b, HexCase::Lower))), + ), _ => return internal_err!("Impossibly got invalid results from digest"), }) } From 82e2bf29ae85457f6213a790d792db42af408524 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 21 Jul 2026 12:38:25 -0600 Subject: [PATCH 5/9] refactor(spark): use shared hex utilities in sha1 and sha2 Replace the per-file hex-encoding lookup tables in Spark's sha1 and sha2 functions with the shared datafusion_common::utils::hex module, continuing the consolidation started for to_hex, hex, and md5. --- datafusion/spark/src/function/hash/sha1.rs | 12 ++------ datafusion/spark/src/function/hash/sha2.rs | 33 ++++++---------------- 2 files changed, 11 insertions(+), 34 deletions(-) diff --git a/datafusion/spark/src/function/hash/sha1.rs b/datafusion/spark/src/function/hash/sha1.rs index dd9009eb8233f..05a224f33f25a 100644 --- a/datafusion/spark/src/function/hash/sha1.rs +++ b/datafusion/spark/src/function/hash/sha1.rs @@ -24,6 +24,7 @@ use datafusion_common::cast::{ as_large_binary_array, }; use datafusion_common::types::{NativeType, logical_string}; +use datafusion_common::utils::hex::{HexCase, encode_bytes}; use datafusion_common::utils::take_function_args; use datafusion_common::{Result, internal_err}; use datafusion_expr::{ @@ -89,18 +90,9 @@ impl ScalarUDFImpl for SparkSha1 { } } -/// Hex encoding lookup table for fast byte-to-hex conversion -const HEX_CHARS_LOWER: &[u8; 16] = b"0123456789abcdef"; - #[inline] fn spark_sha1_digest(value: &[u8]) -> String { - let result = Sha1::digest(value); - let mut s = String::with_capacity(result.len() * 2); - for &b in result.as_slice() { - s.push(HEX_CHARS_LOWER[(b >> 4) as usize] as char); - s.push(HEX_CHARS_LOWER[(b & 0x0f) as usize] as char); - } - s + encode_bytes(&Sha1::digest(value), HexCase::Lower) } fn spark_sha1_impl<'a>(input: impl Iterator>) -> ArrayRef { diff --git a/datafusion/spark/src/function/hash/sha2.rs b/datafusion/spark/src/function/hash/sha2.rs index 38fa0cc643751..541df2957669e 100644 --- a/datafusion/spark/src/function/hash/sha2.rs +++ b/datafusion/spark/src/function/hash/sha2.rs @@ -20,6 +20,7 @@ use arrow::datatypes::{DataType, Int32Type}; use datafusion_common::types::{ NativeType, logical_binary, logical_int32, logical_string, }; +use datafusion_common::utils::hex::{HexCase, encode_bytes}; use datafusion_common::utils::take_function_args; use datafusion_common::{Result, ScalarValue, internal_err}; use datafusion_expr::{ @@ -112,22 +113,22 @@ impl ScalarUDFImpl for SparkSha2 { 224 => { let mut digest = sha2::Sha224::default(); digest.update(bytes); - Some(hex_encode(digest.finalize())) + Some(encode_bytes(&digest.finalize(), HexCase::Lower)) } 0 | 256 => { let mut digest = sha2::Sha256::default(); digest.update(bytes); - Some(hex_encode(digest.finalize())) + Some(encode_bytes(&digest.finalize(), HexCase::Lower)) } 384 => { let mut digest = sha2::Sha384::default(); digest.update(bytes); - Some(hex_encode(digest.finalize())) + Some(encode_bytes(&digest.finalize(), HexCase::Lower)) } 512 => { let mut digest = sha2::Sha512::default(); digest.update(bytes); - Some(hex_encode(digest.finalize())) + Some(encode_bytes(&digest.finalize(), HexCase::Lower)) } _ => None, }; @@ -222,22 +223,22 @@ where (Some(value), Some(224)) => { let mut digest = sha2::Sha224::default(); digest.update(value); - Some(hex_encode(digest.finalize())) + Some(encode_bytes(&digest.finalize(), HexCase::Lower)) } (Some(value), Some(0 | 256)) => { let mut digest = sha2::Sha256::default(); digest.update(value); - Some(hex_encode(digest.finalize())) + Some(encode_bytes(&digest.finalize(), HexCase::Lower)) } (Some(value), Some(384)) => { let mut digest = sha2::Sha384::default(); digest.update(value); - Some(hex_encode(digest.finalize())) + Some(encode_bytes(&digest.finalize(), HexCase::Lower)) } (Some(value), Some(512)) => { let mut digest = sha2::Sha512::default(); digest.update(value); - Some(hex_encode(digest.finalize())) + Some(encode_bytes(&digest.finalize(), HexCase::Lower)) } // Unknown bit-lengths go to null, same as in Spark _ => None, @@ -245,19 +246,3 @@ where .collect::(); Arc::new(array) } - -const HEX_CHARS: [u8; 16] = *b"0123456789abcdef"; - -#[inline] -fn hex_encode>(data: T) -> String { - let bytes = data.as_ref(); - let mut out = Vec::with_capacity(bytes.len() * 2); - for &b in bytes { - let hi = b >> 4; - let lo = b & 0x0F; - out.push(HEX_CHARS[hi as usize]); - out.push(HEX_CHARS[lo as usize]); - } - // SAFETY: out contains only ASCII - unsafe { String::from_utf8_unchecked(out) } -} From f56e985fcf9ff28dfd929e3b9a766f5c06288f8d Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 21 Jul 2026 14:30:05 -0600 Subject: [PATCH 6/9] refactor(functions): use shared hex utilities in encode Add encode_bytes_to_slice to the shared hex module for callers writing into a pre-sized buffer, and migrate the encode() scalar function's last two hex call sites (inner.rs) off the hex crate onto it. This closes the API gap that had left hex_encode_array as the final divergent hex encoder in the workspace. --- datafusion/common/src/utils/hex.rs | 50 ++++++++++++++++++++++ datafusion/functions/src/encoding/inner.rs | 13 +++--- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/datafusion/common/src/utils/hex.rs b/datafusion/common/src/utils/hex.rs index 582ae5f35f1c1..e5cdb7a2e618c 100644 --- a/datafusion/common/src/utils/hex.rs +++ b/datafusion/common/src/utils/hex.rs @@ -80,6 +80,28 @@ pub fn encode_bytes_into(bytes: &[u8], case: HexCase, out: &mut Vec) { } } +/// Writes the hex encoding of `bytes` into `out`. +/// +/// `out` must be exactly `2 * bytes.len()` bytes long. This is for callers +/// that already own a pre-sized buffer (for example a slice of a larger, +/// pre-allocated output array) and want to write directly into it rather +/// than appending to a `Vec`. +/// +/// The length requirement is only checked in debug builds, via +/// `debug_assert_eq!`. In release builds a mismatched `out` length is not +/// an error: only `min(bytes.len(), out.len() / 2)` bytes are encoded, so +/// an `out` that is too short silently drops the remaining input, and an +/// `out` that is too long is left with unwritten, stale bytes at the end. +/// Callers are responsible for sizing `out` correctly. +#[inline] +pub fn encode_bytes_to_slice(bytes: &[u8], case: HexCase, out: &mut [u8]) { + debug_assert_eq!(out.len(), bytes.len() * 2); + let lookup = case.lookup(); + for (&b, chunk) in bytes.iter().zip(out.chunks_exact_mut(2)) { + chunk.copy_from_slice(&lookup[b as usize]); + } +} + /// Returns the hex encoding of `bytes` as an owned `String`. #[inline] pub fn encode_bytes(bytes: &[u8], case: HexCase) -> String { @@ -240,4 +262,32 @@ mod tests { assert_eq!(String::from_utf8(out).unwrap(), encode_bytes(&bytes, case)); } } + + #[test] + fn encode_bytes_to_slice_empty() { + let mut out = []; + encode_bytes_to_slice(&[], HexCase::Lower, &mut out); + assert_eq!(out, []); + } + + #[test] + fn encode_bytes_to_slice_examples() { + let mut out = [0u8; 8]; + encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower, &mut out); + assert_eq!(&out, b"deadbeef"); + + let mut out = [0u8; 8]; + encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Upper, &mut out); + assert_eq!(&out, b"DEADBEEF"); + } + + #[test] + fn encode_bytes_to_slice_agrees_with_encode_bytes() { + let bytes: Vec = (0..=255u8).collect(); + for case in [HexCase::Lower, HexCase::Upper] { + let mut out = vec![0u8; bytes.len() * 2]; + encode_bytes_to_slice(&bytes, case, &mut out); + assert_eq!(String::from_utf8(out).unwrap(), encode_bytes(&bytes, case)); + } + } } diff --git a/datafusion/functions/src/encoding/inner.rs b/datafusion/functions/src/encoding/inner.rs index 027ec8e5e59ab..7c0474f26fc7f 100644 --- a/datafusion/functions/src/encoding/inner.rs +++ b/datafusion/functions/src/encoding/inner.rs @@ -33,7 +33,10 @@ use datafusion_common::{ DataFusionError, Result, ScalarValue, exec_datafusion_err, exec_err, internal_err, not_impl_err, plan_err, types::{NativeType, logical_string}, - utils::take_function_args, + utils::{ + hex::{HexCase, encode_bytes as encode_hex, encode_bytes_to_slice}, + take_function_args, + }, }; use datafusion_expr::{ Coercion, ColumnarValue, Documentation, ScalarFunctionArgs, ScalarUDFImpl, Signature, @@ -369,7 +372,7 @@ impl Encoding { match self { Self::Base64 => BASE64_ENGINE.encode(value), Self::Base64Padded => BASE64_ENGINE_PADDED.encode(value), - Self::Hex => hex::encode(value), + Self::Hex => encode_hex(value, HexCase::Lower), } } @@ -476,11 +479,7 @@ where for v in array.iter() { if let Some(v) = v { let out_len = v.len() * 2; - // The slice is sized to exactly `2 * v.len()`, which is the only - // condition under which `encode_to_slice` can fail, so this cannot - // error. - hex::encode_to_slice(v, &mut values[pos..pos + out_len]) - .map_err(|e| exec_datafusion_err!("Failed to encode to hex: {e}"))?; + encode_bytes_to_slice(v, HexCase::Lower, &mut values[pos..pos + out_len]); pos += out_len; } offsets.push(OutputOffset::usize_as(pos)); From 60a69fda42fb350ca68d3272c137aedbde3e2e53 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 21 Jul 2026 14:34:07 -0600 Subject: [PATCH 7/9] refactor(common): address review feedback on hex utilities Reword the write_digits doc comment to describe the actual borrow conflict (the v == 0 early return, not a return inside the loop), drop the unreachable & 0xF mask in build_lookup, add doctests to the public encode_bytes/encode_u64/encode_bytes_to_slice functions plus a less internal module doc comment, replace the tautological encode_bytes/encode_bytes_into agreement test with one that pins buffer-reuse behavior in encode_u64, and cover the untested lowercase path of hex_encode_bytes in the Spark hex function. --- datafusion/common/src/utils/hex.rs | 70 +++++++++++++++++------ datafusion/spark/src/function/math/hex.rs | 17 +++++- 2 files changed, 69 insertions(+), 18 deletions(-) diff --git a/datafusion/common/src/utils/hex.rs b/datafusion/common/src/utils/hex.rs index e5cdb7a2e618c..f8f5985e5dd74 100644 --- a/datafusion/common/src/utils/hex.rs +++ b/datafusion/common/src/utils/hex.rs @@ -15,12 +15,13 @@ // specific language governing permissions and limitations // under the License. -//! Hex encoding shared across the workspace. +//! Hex encoding of bytes and integers. //! -//! `to_hex`, Spark's `hex`, and the digest functions (`md5`, `sha1`, `sha2`) -//! all need the same conversion. Keeping one implementation here avoids the -//! per-crate lookup tables that previously diverged in both speed and case -//! handling. +//! [`encode_bytes`] and [`encode_bytes_into`] encode a byte slice into an +//! owned `String` or an appended `Vec`, respectively; [`encode_bytes_to_slice`] +//! writes into a caller-provided, pre-sized buffer. [`encode_u64`] encodes an +//! integer, trimming leading zeros. All four take a [`HexCase`] to choose +//! between lowercase and uppercase digits. /// Case of the emitted hex digits. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -43,7 +44,7 @@ const fn build_lookup(digits: &[u8; 16]) -> [[u8; 2]; 256] { let mut table = [[0u8; 2]; 256]; let mut i = 0; while i < 256 { - table[i][0] = digits[(i >> 4) & 0xF]; + table[i][0] = digits[i >> 4]; table[i][1] = digits[i & 0xF]; i += 1; } @@ -93,6 +94,16 @@ pub fn encode_bytes_into(bytes: &[u8], case: HexCase, out: &mut Vec) { /// an `out` that is too short silently drops the remaining input, and an /// `out` that is too long is left with unwritten, stale bytes at the end. /// Callers are responsible for sizing `out` correctly. +/// +/// # Example +/// +/// ``` +/// use datafusion_common::utils::hex::{HexCase, encode_bytes_to_slice}; +/// +/// let mut out = [0u8; 8]; +/// encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower, &mut out); +/// assert_eq!(&out, b"deadbeef"); +/// ``` #[inline] pub fn encode_bytes_to_slice(bytes: &[u8], case: HexCase, out: &mut [u8]) { debug_assert_eq!(out.len(), bytes.len() * 2); @@ -103,6 +114,15 @@ pub fn encode_bytes_to_slice(bytes: &[u8], case: HexCase, out: &mut [u8]) { } /// Returns the hex encoding of `bytes` as an owned `String`. +/// +/// # Example +/// +/// ``` +/// use datafusion_common::utils::hex::{HexCase, encode_bytes}; +/// +/// assert_eq!(encode_bytes(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower), "deadbeef"); +/// assert_eq!(encode_bytes(&[0xde, 0xad, 0xbe, 0xef], HexCase::Upper), "DEADBEEF"); +/// ``` #[inline] pub fn encode_bytes(bytes: &[u8], case: HexCase) -> String { let mut out = Vec::with_capacity(bytes.len() * 2); @@ -119,6 +139,19 @@ pub fn encode_bytes(bytes: &[u8], case: HexCase) -> String { /// Signed values should be cast with `as u64`, which yields the two's /// complement representation that both `to_hex` and Spark's `hex` produce for /// negative input. +/// +/// # Example +/// +/// The caller owns the buffer and can reuse it across calls; each call +/// returns a fresh subslice of it, borrowed for as long as `buf` is: +/// +/// ``` +/// use datafusion_common::utils::hex::{HexCase, encode_u64}; +/// +/// let mut buf = [0u8; 16]; +/// assert_eq!(encode_u64(0xAB, HexCase::Lower, &mut buf), b"ab"); +/// assert_eq!(encode_u64(0, HexCase::Lower, &mut buf), b"0"); +/// ``` #[inline] pub fn encode_u64(v: u64, case: HexCase, buf: &mut [u8; 16]) -> &[u8] { let start = write_digits(v, case, buf); @@ -129,8 +162,10 @@ pub fn encode_u64(v: u64, case: HexCase, buf: &mut [u8; 16]) -> &[u8] { /// first digit. /// /// Split out from [`encode_u64`] so the mutable borrow of `buf` ends before the -/// returned slice reborrows it; a conditional `return &buf[..]` inside the loop -/// body would outlive the writes that follow it. +/// returned slice reborrows it. The `v == 0` case returns early; if that early +/// return instead reborrowed `buf` as `&buf[..]` inline in [`encode_u64`], the +/// borrow checker would extend that reborrow over the rest of the function +/// body, conflicting with the mutable writes on the non-zero path below. #[inline] fn write_digits(v: u64, case: HexCase, buf: &mut [u8; 16]) -> usize { if v == 0 { @@ -254,20 +289,21 @@ mod tests { } #[test] - fn encode_bytes_agrees_with_encode_bytes_into() { - let bytes: Vec = (0..=255u8).collect(); - for case in [HexCase::Lower, HexCase::Upper] { - let mut out = Vec::new(); - encode_bytes_into(&bytes, case, &mut out); - assert_eq!(String::from_utf8(out).unwrap(), encode_bytes(&bytes, case)); - } + fn encode_u64_reused_buffer_leaks_no_stale_digits() { + let mut buf = [0u8; 16]; + assert_eq!( + encode_u64(u64::MAX, HexCase::Lower, &mut buf), + b"ffffffffffffffff" + ); + assert_eq!(encode_u64(0, HexCase::Lower, &mut buf), b"0"); + assert_eq!(encode_u64(0xAB, HexCase::Lower, &mut buf), b"ab"); } #[test] fn encode_bytes_to_slice_empty() { - let mut out = []; + let mut out: [u8; 0] = []; encode_bytes_to_slice(&[], HexCase::Lower, &mut out); - assert_eq!(out, []); + assert_eq!(out, [] as [u8; 0]); } #[test] diff --git a/datafusion/spark/src/function/math/hex.rs b/datafusion/spark/src/function/math/hex.rs index c9814a94e82dd..1f505fda21b6f 100644 --- a/datafusion/spark/src/function/math/hex.rs +++ b/datafusion/spark/src/function/math/hex.rs @@ -335,7 +335,7 @@ mod test { use std::sync::Arc; use arrow::array::{ - BinaryArray, DictionaryArray, Int32Array, Int64Array, StringArray, + Array, BinaryArray, DictionaryArray, Int32Array, Int64Array, StringArray, }; use arrow::{ array::{ @@ -458,6 +458,21 @@ mod test { } } + #[test] + fn test_hex_encode_bytes_lowercase() { + // Every in-repo caller of `hex_encode_bytes` goes through `spark_hex`, + // which always passes `lowercase = false`. The `lowercase = true` path + // is reachable only via `spark_sha2_hex`, which has no in-workspace + // caller, so it otherwise has no coverage. Drive it directly here. + let input = StringArray::from(vec![Some("hi"), Some("bye"), None, Some("rust")]); + let result = super::hex_encode_bytes(input.iter(), true, input.len()).unwrap(); + let result = as_string_array(&result); + + let expected = + StringArray::from(vec![Some("6869"), Some("627965"), None, Some("72757374")]); + assert_eq!(result, &expected); + } + #[test] fn test_spark_hex_binary_round_trip_all_bytes() { // Single-row binary input containing every byte value, encoded in From 5e7e2112731a9e0810f4555a6fbf5b1b2636ca8e Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 21 Jul 2026 15:23:15 -0600 Subject: [PATCH 8/9] perf(common): force inlining of hex encoders across crate boundaries The encode entry points are called once per row from datafusion-functions and datafusion-spark. Plain #[inline] left them out-of-line across the crate boundary, which cost 2-3% on Spark's byte paths and up to 18% on to_hex's i32 path relative to the pre-refactor local implementations. --- datafusion/common/src/utils/hex.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/datafusion/common/src/utils/hex.rs b/datafusion/common/src/utils/hex.rs index f8f5985e5dd74..6e53c51c67c71 100644 --- a/datafusion/common/src/utils/hex.rs +++ b/datafusion/common/src/utils/hex.rs @@ -73,7 +73,7 @@ impl HexCase { /// /// Allocates only through `out`'s own growth. Callers that must bound or guard /// that growth should reserve capacity in `out` before calling. -#[inline] +#[inline(always)] pub fn encode_bytes_into(bytes: &[u8], case: HexCase, out: &mut Vec) { let lookup = case.lookup(); for &byte in bytes { @@ -104,7 +104,7 @@ pub fn encode_bytes_into(bytes: &[u8], case: HexCase, out: &mut Vec) { /// encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower, &mut out); /// assert_eq!(&out, b"deadbeef"); /// ``` -#[inline] +#[inline(always)] pub fn encode_bytes_to_slice(bytes: &[u8], case: HexCase, out: &mut [u8]) { debug_assert_eq!(out.len(), bytes.len() * 2); let lookup = case.lookup(); @@ -152,7 +152,7 @@ pub fn encode_bytes(bytes: &[u8], case: HexCase) -> String { /// assert_eq!(encode_u64(0xAB, HexCase::Lower, &mut buf), b"ab"); /// assert_eq!(encode_u64(0, HexCase::Lower, &mut buf), b"0"); /// ``` -#[inline] +#[inline(always)] pub fn encode_u64(v: u64, case: HexCase, buf: &mut [u8; 16]) -> &[u8] { let start = write_digits(v, case, buf); &buf[start..] @@ -166,7 +166,7 @@ pub fn encode_u64(v: u64, case: HexCase, buf: &mut [u8; 16]) -> &[u8] { /// return instead reborrowed `buf` as `&buf[..]` inline in [`encode_u64`], the /// borrow checker would extend that reborrow over the rest of the function /// body, conflicting with the mutable writes on the non-zero path below. -#[inline] +#[inline(always)] fn write_digits(v: u64, case: HexCase, buf: &mut [u8; 16]) -> usize { if v == 0 { buf[15] = b'0'; From f30b8e84df65230c6980b93fedcb442ba180d2b7 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Wed, 22 Jul 2026 11:26:31 -0600 Subject: [PATCH 9/9] refactor(common): return an error from encode_bytes_to_slice on a length mismatch A debug_assert let release builds silently encode only part of the input when the output buffer was the wrong size. Check unconditionally and return an internal error instead. Also trims the write_digits doc comment to the borrow-checker constraint it needs to explain. --- datafusion/common/src/utils/hex.rs | 76 +++++++++++++++------- datafusion/functions/src/encoding/inner.rs | 2 +- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/datafusion/common/src/utils/hex.rs b/datafusion/common/src/utils/hex.rs index 6e53c51c67c71..eec55a5e08436 100644 --- a/datafusion/common/src/utils/hex.rs +++ b/datafusion/common/src/utils/hex.rs @@ -23,6 +23,9 @@ //! integer, trimming leading zeros. All four take a [`HexCase`] to choose //! between lowercase and uppercase digits. +use crate::Result; +use crate::error::_internal_err; + /// Case of the emitted hex digits. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum HexCase { @@ -83,17 +86,12 @@ pub fn encode_bytes_into(bytes: &[u8], case: HexCase, out: &mut Vec) { /// Writes the hex encoding of `bytes` into `out`. /// -/// `out` must be exactly `2 * bytes.len()` bytes long. This is for callers -/// that already own a pre-sized buffer (for example a slice of a larger, -/// pre-allocated output array) and want to write directly into it rather -/// than appending to a `Vec`. +/// This is for callers that already own a pre-sized buffer (for example a +/// slice of a larger, pre-allocated output array) and want to write directly +/// into it rather than appending to a `Vec`. /// -/// The length requirement is only checked in debug builds, via -/// `debug_assert_eq!`. In release builds a mismatched `out` length is not -/// an error: only `min(bytes.len(), out.len() / 2)` bytes are encoded, so -/// an `out` that is too short silently drops the remaining input, and an -/// `out` that is too long is left with unwritten, stale bytes at the end. -/// Callers are responsible for sizing `out` correctly. +/// Returns an internal error if `out` is not exactly `2 * bytes.len()` bytes +/// long, rather than silently encoding only part of the input. /// /// # Example /// @@ -101,16 +99,24 @@ pub fn encode_bytes_into(bytes: &[u8], case: HexCase, out: &mut Vec) { /// use datafusion_common::utils::hex::{HexCase, encode_bytes_to_slice}; /// /// let mut out = [0u8; 8]; -/// encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower, &mut out); +/// encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower, &mut out)?; /// assert_eq!(&out, b"deadbeef"); +/// # Ok::<(), datafusion_common::DataFusionError>(()) /// ``` #[inline(always)] -pub fn encode_bytes_to_slice(bytes: &[u8], case: HexCase, out: &mut [u8]) { - debug_assert_eq!(out.len(), bytes.len() * 2); +pub fn encode_bytes_to_slice(bytes: &[u8], case: HexCase, out: &mut [u8]) -> Result<()> { + let expected = bytes.len() * 2; + if out.len() != expected { + return _internal_err!( + "hex output buffer is {} bytes, expected {expected}", + out.len() + ); + } let lookup = case.lookup(); for (&b, chunk) in bytes.iter().zip(out.chunks_exact_mut(2)) { chunk.copy_from_slice(&lookup[b as usize]); } + Ok(()) } /// Returns the hex encoding of `bytes` as an owned `String`. @@ -162,10 +168,7 @@ pub fn encode_u64(v: u64, case: HexCase, buf: &mut [u8; 16]) -> &[u8] { /// first digit. /// /// Split out from [`encode_u64`] so the mutable borrow of `buf` ends before the -/// returned slice reborrows it. The `v == 0` case returns early; if that early -/// return instead reborrowed `buf` as `&buf[..]` inline in [`encode_u64`], the -/// borrow checker would extend that reborrow over the rest of the function -/// body, conflicting with the mutable writes on the non-zero path below. +/// returned slice reborrows it. #[inline(always)] fn write_digits(v: u64, case: HexCase, buf: &mut [u8; 16]) -> usize { if v == 0 { @@ -300,30 +303,55 @@ mod tests { } #[test] - fn encode_bytes_to_slice_empty() { + fn encode_bytes_to_slice_empty() -> Result<()> { let mut out: [u8; 0] = []; - encode_bytes_to_slice(&[], HexCase::Lower, &mut out); + encode_bytes_to_slice(&[], HexCase::Lower, &mut out)?; assert_eq!(out, [] as [u8; 0]); + Ok(()) } #[test] - fn encode_bytes_to_slice_examples() { + fn encode_bytes_to_slice_examples() -> Result<()> { let mut out = [0u8; 8]; - encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower, &mut out); + encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower, &mut out)?; assert_eq!(&out, b"deadbeef"); let mut out = [0u8; 8]; - encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Upper, &mut out); + encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Upper, &mut out)?; assert_eq!(&out, b"DEADBEEF"); + Ok(()) } #[test] - fn encode_bytes_to_slice_agrees_with_encode_bytes() { + fn encode_bytes_to_slice_agrees_with_encode_bytes() -> Result<()> { let bytes: Vec = (0..=255u8).collect(); for case in [HexCase::Lower, HexCase::Upper] { let mut out = vec![0u8; bytes.len() * 2]; - encode_bytes_to_slice(&bytes, case, &mut out); + encode_bytes_to_slice(&bytes, case, &mut out)?; assert_eq!(String::from_utf8(out).unwrap(), encode_bytes(&bytes, case)); } + Ok(()) + } + + #[test] + fn encode_bytes_to_slice_rejects_wrong_length() { + // Too short: the old `debug_assert` let release builds silently drop + // the remaining input. + let mut short = [0u8; 6]; + let err = + encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower, &mut short) + .unwrap_err(); + assert!( + err.message() + .contains("hex output buffer is 6 bytes, expected 8"), + "unexpected message: {err}" + ); + + // Too long: would have left stale bytes at the tail. + let mut long = [0u8; 10]; + assert!( + encode_bytes_to_slice(&[0xde, 0xad, 0xbe, 0xef], HexCase::Lower, &mut long) + .is_err() + ); } } diff --git a/datafusion/functions/src/encoding/inner.rs b/datafusion/functions/src/encoding/inner.rs index 7c0474f26fc7f..472a7f8820d7f 100644 --- a/datafusion/functions/src/encoding/inner.rs +++ b/datafusion/functions/src/encoding/inner.rs @@ -479,7 +479,7 @@ where for v in array.iter() { if let Some(v) = v { let out_len = v.len() * 2; - encode_bytes_to_slice(v, HexCase::Lower, &mut values[pos..pos + out_len]); + encode_bytes_to_slice(v, HexCase::Lower, &mut values[pos..pos + out_len])?; pos += out_len; } offsets.push(OutputOffset::usize_as(pos));