apache / apache/datafusion-python

Quarantine the greedy ForeignExecutionPlan claim so it cannot be copied by accident

Đang mở
#1,725 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
documentation rust
Ngôn ngữ chính
Python
Star
604
Fork
174
Merge trung bình
1 ngày 7 giờ
Pull request đã merge (30 ngày)
4

Mô tả

`examples/datafusion-ffi-example/src/physical_extension_codec.rs:150` claims `node.is::() || node.is::()`. The second arm takes every other library's nodes, which `extension-guide/checklist.md` tells authors never to do.

**This is not a bug to fix.** It is G2 in #1719: the arm is load-bearing for upstream defect apache/datafusion#25152, and narrowing it to `DataSourceExec` alone makes 31 of the 51 tests in `datafusion-ffi-query-planner-example` fail. The object registry in the same file is downstream of it — once you claim a node you cannot introspect, there is nothing to write down about it, so parking it is all that is left.

The goal is to make it impossible to copy by accident while it stays. Proposal: move the statics and helpers into a new `src/foreign_plan_workaround.rs` behind three deliberately blunt functions — `claims()`, `park()`, `take()`. Nobody reads `foreign_plan_workaround::park(node, buf)` and thinks they are looking at serialization. The 25-line explanation currently buried inside a function body at lines 125-149 becomes the module `//!` doc, with three greppable fields:

```rust
//! # NOT A PATTERN
//! **Blocked on:**
//! **Delete when:** `FFI_PlanProperties` carries `scheduling_type`, or
//! `ForeignExecutionPlan` gains a reachable `try_to_proto`.
//! **Copying this will:** claim every other library's plan nodes, and produce
//! payloads that decode only in the writing process, exactly once each.
```

One honesty note belongs in that doc: the `DataSourceExec` arm *could* be durable and is not, because the registry has to exist for the `ForeignExecutionPlan` arm regardless. Splitting the two arms across two wire formats costs real code and removes nothing. Saying so lets the guide claim the registry cannot be removed without also claiming every byte of it is forced.

**Do not rename `MyPhysicalExtensionCodec`** — 21 call sites plus two doc references, and the module doc already carries the label.

**Done when:** `grep -rn "NOT A PATTERN" examples/` returns exactly one site, and `cargo clippy --no-deps --all-targets` is clean. No test changes expected; this is code motion plus documentation.

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

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Hướng nghiên cứu

Start with examples/datafusion-ffi-example/src/physical_extension_codec.rs:125-150 and the related guidance in extension-guide/checklist.md. Trace the statics and helpers used by MyPhysicalExtensionCodec, then isolate the workaround behind claims(), park(), and take() in src/foreign_plan_workaround.rs with the specified module documentation. Done means grep finds exactly one “NOT A PATTERN” in examples/ and cargo clippy --no-deps --all-targets is clean.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
rust
Lĩnh vực
backend-api-design
Loại issue
Tái cấu trúc
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
72/100

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.