andygrove opened a new issue, #23811: URL: https://github.com/apache/datafusion/issues/23811
### Is your feature request related to a problem or challenge? Follow-on from https://github.com/apache/datafusion/pull/23766, where @alamb [asked](https://github.com/apache/datafusion/pull/23766#discussion_r3627108782) 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: ```rust 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`. -- 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]
