apache / apache/datafusion

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

Abierto
#22,331 5 comentarios 0 reacciones 1 asignado Reclamado por @Amogh-2404 Ver en GitHub
enhancement ffi functions
Lenguaje dominante
Rust
Estrellas
9.3k
Forks
2.4k
Merge medio
3 d 11 h
PR fusionados (30 d)
362

Descripción

## 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.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.