mbutrovich commented on PR #4911: URL: https://github.com/apache/datafusion-comet/pull/4911#issuecomment-4985656784
Both this and #4926 edit the shared `make_decimal_array` signature in `utils.rs`. #4911 changes it to `make_decimal_array<F: Fn(i128)->i128>(... f: F)`; #4926 changes the same line to `f: impl Fn(i128)->i128`. These two edits conflict on merge, and worse the two PRs solve the same problem two different ways: #4911 dispatches on a compile-time `EXP` const (`dispatch_pow10!` + `decimal_floor_pow10`), while #4926 does a per-row runtime `i64::try_from` branch inside `decimal_ceil_f` (`ceil.rs:79`). Floor and ceil are the same operation with the sign flipped and should share one mechanism. Suggested change: land one mechanism and apply it to both. The `dispatch_pow10!` macro already lives in `utils.rs` and is written generically, so `spark_ceil` should adopt it with a `decimal_ceil_pow10::<EXP>` mirror of `decimal_floor_pow10`. Coordinate the `make_decimal_array` signature so only one of the two forms lands (`impl Fn` and `<F: Fn>` are equivalent here, pick one). Sequence the two PRs so the second rebases onto the first. -- 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]
