andygrove opened a new issue, #26303: URL: https://github.com/apache/datafusion/issues/26303
### Is your feature request related to a problem or challenge? `lpad` and `rpad` contain four copies of the same padding loop: - [`lpad_impl` (unicode/lpad.rs#L366-L513)](https://github.com/apache/datafusion/blob/0d79252f61bd8777990b2749c216ffd07079c30d/datafusion/functions/src/unicode/lpad.rs#L366-L513) has one loop for an explicit fill array (about L376-L445) and a near-identical one for the default space fill (about L447-L505). Each repeats the `i32::MAX` check, negative-length clamp, zero-length shortcut, ASCII fast path and char-boundary fallback (nesting depth 7, cognitive complexity 111). - [`rpad_impl` (unicode/rpad.rs#L365-L516)](https://github.com/apache/datafusion/blob/0d79252f61bd8777990b2749c216ffd07079c30d/datafusion/functions/src/unicode/rpad.rs#L365-L516) is the mirror image. The only differences are whether the padding is written before or after the string, and the function name in the error text. The copies match one for one today, but any fix to length validation or Unicode handling has to be made in four places. ### Describe the solution you'd like - Treat the default-space case as the fill case with `fill = " "` (monomorphized, so the hot loop is unchanged), removing the second loop in each function. - Extract a shared `parse_target_len(target_len, fn_name)` for the `i32::MAX` / negative clamp. - Move a single pad kernel, generic over the pad side (e.g. a `PadSide::{Left, Right}` const generic), into a shared module under `datafusion/functions/src/unicode`, leaving `lpad.rs`/`rpad.rs` as thin wrappers. These functions were recently optimized (#20277, #20655, #21060), so the change must keep the ASCII fast path and be checked against the existing pad benchmarks. ### Describe alternatives you've considered Only merge the with-fill and no-fill loops within each function, without sharing code between lpad and rpad. ### Additional context Line references are pinned to commit 0d79252f61bd. -- 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]
