Is your feature request related to a problem or challenge?
Follow-on from #23766, where @alamb asked whether to_hex's integer-to-hex code should live in the common crate alongside the byte encoders.
#23766 moved the low-level hex primitives into datafusion_common::utils::hex, including encode_u64, which every integer width funnels into. What stayed behind in datafusion/functions/src/string/to_hex.rs is the Arrow-facing layer:
trait ToHex: ArrowNativeType {
fn write_hex(self, buf: &mut [u8; 16]) -> &[u8];
}
plus eight macro-generated impls that widen each integer type (self as i64 as u64 for signed, self as u64 for unsigned) before delegating, and to_hex_scalar, which wraps the result in a String.
So the split today is: datafusion-common owns hex encoding of a u64, and datafusion-functions owns the mapping from Arrow integer types onto that.
Describe the solution you'd like
Decide whether the Arrow-type dispatch belongs in datafusion-common too, and if so move the ToHex trait and its impls there.
Arguments for: it is generic, reusable, and has no to_hex-specific logic. Spark's hex accepts numeric input coerced to Int64 and does its own num as u64 cast, so it would be a second consumer.
Arguments against: datafusion-common is published and semver-constrained, and #23766 already added four public functions to it. The widening rules are the part most likely to want per-dialect variation — a caller wanting to_hex(-1::TINYINT) to render as ff rather than ffffffffffffffff needs different behaviour, and baking one choice into common makes that harder.
to_hex_scalar allocating a String is tracked separately in #23810.
Describe alternatives you've considered
Leaving it where it is. The current boundary — common owns the encoder, each function crate owns its own type mapping and dialect semantics — is defensible, and this issue may reasonably close as "working as intended".
Additional context
Relevant code: datafusion/functions/src/string/to_hex.rs, datafusion/common/src/utils/hex.rs.
Is your feature request related to a problem or challenge?
Follow-on from #23766, where @alamb asked whether
to_hex's integer-to-hex code should live in the common crate alongside the byte encoders.#23766 moved the low-level hex primitives into
datafusion_common::utils::hex, includingencode_u64, which every integer width funnels into. What stayed behind indatafusion/functions/src/string/to_hex.rsis the Arrow-facing layer:plus eight macro-generated impls that widen each integer type (
self as i64 as u64for signed,self as u64for unsigned) before delegating, andto_hex_scalar, which wraps the result in aString.So the split today is:
datafusion-commonowns hex encoding of au64, anddatafusion-functionsowns the mapping from Arrow integer types onto that.Describe the solution you'd like
Decide whether the Arrow-type dispatch belongs in
datafusion-commontoo, and if so move theToHextrait and its impls there.Arguments for: it is generic, reusable, and has no
to_hex-specific logic. Spark'shexaccepts numeric input coerced toInt64and does its ownnum as u64cast, so it would be a second consumer.Arguments against:
datafusion-commonis published and semver-constrained, and #23766 already added four public functions to it. The widening rules are the part most likely to want per-dialect variation — a caller wantingto_hex(-1::TINYINT)to render asffrather thanffffffffffffffffneeds different behaviour, and baking one choice into common makes that harder.to_hex_scalarallocating aStringis tracked separately in #23810.Describe alternatives you've considered
Leaving it where it is. The current boundary — common owns the encoder, each function crate owns its own type mapping and dialect semantics — is defensible, and this issue may reasonably close as "working as intended".
Additional context
Relevant code:
datafusion/functions/src/string/to_hex.rs,datafusion/common/src/utils/hex.rs.