apache / apache/datafusion

FFI: `FFI_AggregateUDF` silently drops producer overrides of defaulted trait methods

Aperta
#22,331 5 commenti 0 reazioni 1 assegnatario Rivendicata da @Amogh-2404 Vedi su GitHub
enhancement ffi functions
Lingua principale
Rust
Stelle
9.3k
Fork
2.4k
Merge medio
3g 11h
PR unite (30g)
362

Descrizione

## 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` 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-`.
- 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 `datafusion-ffi` skill audit. See `.ai/skills/datafusion-ffi/SKILL.md` §"Method coverage" and §"Known gaps to close" (originated in PR #22327). 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.

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.