timsaucer opened a new issue, #22331:
URL: https://github.com/apache/datafusion/issues/22331

   ## Gap
   
   `FFI_AggregateUDF` in `datafusion/ffi/src/udaf/mod.rs` does not plumb 
several defaulted methods of `AggregateUDFImpl`. Producer overrides are 
silently lost on the consumer side.
   
   ## Missing methods
   
   - `display_name`
   - `schema_name`
   - `human_display`
   - `window_function_schema_name`
   - `window_function_display_name`
   - `simplify`
   - `simplify_expr_op_literal`
   - `reverse_udf` / `reverse_expr`
   - `is_descending`
   - `value_from_stats`
   - `default_value`
   - `supports_null_handling_clause`
   - `supports_within_group_clause`
   - `set_monotonicity`
   - `documentation`
   
   ## Why it matters
   
   **Severity: critical.** `value_from_stats` enables statistics-driven 
shortcuts (e.g. `MIN`/`MAX` from precomputed stats) — silent loss forces full 
re-aggregation across the FFI boundary. `default_value` affects empty-group 
correctness. `supports_null_handling_clause` / `supports_within_group_clause` 
change accepted SQL surface area. `simplify` / `simplify_expr_op_literal` / 
`reverse_*` are optimizer hooks. SQL output naming wrong without `display_name` 
/ `schema_name`.
   
   ## Implementation notes
   
   - Plumb each as a plain `unsafe extern \"C\" fn`; wrapper body calls the 
trait method on inner `Arc<dyn AggregateUDFImpl>` and dispatch handles 
override-or-default.
   - Methods that ship `Expr` (`simplify`, `simplify_expr_op_literal`, 
`reverse_expr`) require the embedded `FFI_LogicalExtensionCodec`.
   - Layout change → `api change` label, target `main` only, no back-port to 
`branch-<major>`.
   - Add unit tests (local-bypass + `mock_foreign_marker_id` forced-foreign) 
**and** integration tests under `datafusion/ffi/tests/` for any method shipping 
non-trivial FFI types.
   
   ---
   
   Generated from an audit performed for PR #22327 (datafusion-ffi agent 
skill). If a PR addressing this finds any item to be a false positive (e.g., a 
method intentionally omitted for a documented reason), please also propose an 
update to the `datafusion-ffi` skill so future audits do not re-flag it.


-- 
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]

Reply via email to