kosiew commented on code in PR #22388:
URL: https://github.com/apache/datafusion/pull/22388#discussion_r3301846504
##########
datafusion/spark/src/function/string/format_string.rs:
##########
@@ -891,23 +891,82 @@ fn unsigned_to_char(value: u64) -> Result<char> {
codepoint_to_char(codepoint)
}
-/// Convert a non-null integer scalar to a [`char`] for the `%c` conversion.
-fn integer_scalar_to_char(scalar: &ScalarValue) -> Result<char> {
- match scalar {
- ScalarValue::Int8(Some(value)) => signed_to_char(*value as i64),
- ScalarValue::Int16(Some(value)) => signed_to_char(*value as i64),
- ScalarValue::Int32(Some(value)) => signed_to_char(*value as i64),
- ScalarValue::Int64(Some(value)) => signed_to_char(*value),
- ScalarValue::UInt8(Some(value)) => unsigned_to_char(*value as u64),
- ScalarValue::UInt16(Some(value)) => unsigned_to_char(*value as u64),
- ScalarValue::UInt32(Some(value)) => unsigned_to_char(*value as u64),
- ScalarValue::UInt64(Some(value)) => unsigned_to_char(*value),
- _ => datafusion_common::internal_err!(
- "integer_scalar_to_char expects a non-null integer scalar, got
{scalar:?}"
- ),
+/// Normalizes integer scalar payloads while preserving Spark formatting
semantics:
+/// signed values format as decimal for `%d` / `%s` / `%c`, but use their
original
+/// bit width for `%x` / `%o` via `unsigned_bits`.
+#[derive(Debug, Clone, Copy)]
+enum IntegerValue {
Review Comment:
💡 Good idea!
I will replace this with a narrow local trait implemented directly for the
primitive integer types (`i8`/`i16`/`i32`/`i64` and `u8`/`u16`/`u32`/`u64`).
The shared `format_integer` helper can then be generic over that trait, while
each primitive supplies the few operations where signedness/width matter:
decimal formatting, `%x`/`%o` unsigned bit representation, `%c`, and `%s`
string rendering.
This should preserve the current behavior and the added width/null
regression coverage, but avoid the intermediary enum and make the dispatch
intent more direct.
--
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]