[I] Reduce boilerplate in native expression implementations [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove opened a new issue, #4708:
URL: https://github.com/apache/datafusion-comet/issues/4708

   ## Is your feature request related to a problem or challenge?
   
   The native expression implementations in `native/spark-expr` and `native/core` contain a large amount of mechanical boilerplate that is copy-pasted across most `PhysicalExpr` and `ScalarUDFImpl` implementations. An audit of roughly 30 `PhysicalExpr` impls and 21 `ScalarUDFImpl` impls found several recurring patterns that add up to an estimated 1,500 to 2,000 lines of code that carry no real information and that contributors must re-derive by hand for every new expression.
   
   The repeated patterns are:
   
   1. **Hand-written `impl Hash` / `impl PartialEq`** that simply recurse over every field. Around 24 files do this; many could be a plain `#[derive(Hash, PartialEq, Eq)]`. `string_funcs/contains.rs` already uses derive, so the codebase is just inconsistent. Example: `math_funcs/negative.rs:49-60`.
   
   2. **Mechanical `PhysicalExpr` trait methods**: `children()`, `with_new_children()`, the `fmt_sql()` to `Display::fmt` forwarder, and pass-through `data_type()` / `nullable()`. The `fmt_sql` forwarder alone is duplicated verbatim in ~24 files. Note that `fmt_sql` is a required method on the upstream trait, so this cannot be solved with a trait default and needs a Comet-side macro. Example: `math_funcs/negative.rs:189-251`.
   
   3. **`ScalarUDFImpl` scaffolding**: a struct with `signature` + `aliases`, a `Default`, a `new()`, then `name()` / `signature()` / `return_type()` (often a constant) and an `invoke_with_args()` that forwards to a free function. ~21 files, ~50 lines each. Examples: `array_funcs/size.rs:74-90`, `datetime_funcs/hours.rs:58-101`.
   
   4. **Scalar vs array dispatch**: the `match ColumnarValue { Array(a) => .., Scalar(s) => .. }` wrap/unwrap appears in ~8 functions. Example: `json_funcs/json_array_length.rs:71-85`.
   
   ## Describe the solution you'd like
   
   Introduce lightweight framework helpers to remove this boilerplate, rolled out incrementally so each step is independently reviewable and behavior-preserving:
   
   - [ ] **Replace hand-written `Hash` / `PartialEq` with `#[derive]`** where the manual impl covers every field with no transformation (the safe subset, roughly 13 structs). Impls that intentionally exclude a field (`registry`, `query_context`, `task_context`) or hold a non-deriving field (`Regex`, `Mutex`, `AtomicI64`, `Signature`) stay hand-written.
   - [ ] **A `macro_rules!` for the mechanical `PhysicalExpr` methods** (`children`, `with_new_children`, `fmt_sql`, optional `data_type` / `nullable` pass-through) generated from a field list.
   - [ ] **A declarative macro for `ScalarUDFImpl`** that generates the struct, `Default`, `new()`, and the trivial trait methods, leaving the author to write only the invoke logic.
   - [ ] **A `scalar_array_dispatch` helper** for the array/scalar wrap-unwrap pattern.
   
   Each item should land as its own PR.
   
   ## Describe alternatives you've considered
   
   - A proc-macro crate (`spark-expr-macros`) instead of `macro_rules!`. This is more powerful for reconstructing structs in `with_new_children`, but adds a build dependency and complexity. Defer unless `macro_rules!` proves insufficient.
   - Pushing trait defaults upstream into DataFusion. Not viable for `fmt_sql` in the short term since the trait lives in our dependency.
   
   ## Additional context
   
   This is an umbrella tracking issue. Start with the derive cleanup since it is pure, low-risk, and requires no new abstraction.
   


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Reduce boilerplate in native expression implementations [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove closed issue #4708: Reduce boilerplate in native expression implementations
URL: https://github.com/apache/datafusion-comet/issues/4708


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org