apache / apache/datafusion

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

Đang mở
#22,331 5 bình luận 0 reaction 1 người được giao Được @Amogh-2404 nhận Xem trên GitHub
enhancement ffi functions
Ngôn ngữ chính
Rust
Star
9.3k
Fork
2.4k
Merge trung bình
3 ngày 11 giờ
Pull request đã merge (30 ngày)
360

Mô tả

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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.