apache / apache/datafusion-comet
Make native contrib scan dispatch generic (remove per-contrib arms from core Rust)
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 198
Description
Follow-up from review of #4952 ([thread](https://github.com/apache/datafusion-comet/pull/4952#discussion_r_planner_rs)).
### Background
On the JVM side, #4952 makes contrib scan wiring fully generic: `CometScanContrib` is a `ServiceLoader`-discovered SPI, core holds no compile-time reference to any contrib, and a default build discovers nothing.
The native side is *nearly* there. Core's dispatcher arm is already generic — it matches `OpStruct::ContribScan` and routes on `type_url`, and every Delta-specific concern (the type name, the decode, the planning) lives inside a `#[cfg(feature = "contrib-delta")]` module:
```rust
OpStruct::ContribScan(contrib) => {
#[cfg(feature = "contrib-delta")]
if let Some(result) = delta_scan::try_plan_contrib_scan(self, spark_plan, contrib) {
return result;
}
Err(GeneralError(format!(
"Received a contrib_scan operator (type_url: {}) but core was built without a \
contrib that handles it. ...", contrib.type_url)))
}
```
What remains is that core still *names* each contrib: one `#[cfg]`-gated call per contrib, growing by a line as contribs are added. A registry of `type_url -> handler` that contribs populate would remove even that.
### Two paths (from the review)
1. **A true ServiceLoader-style system** — dynamic discovery and loading of an extension at runtime. "There may be dragons along this path." Reference: https://nullderef.com/blog/plugin-dynload/
2. **Statically linked, independently built crates** — each contrib crate builds on its own and registers into core's dispatch table. Likely needs crate-level refactoring to avoid a `core -> contrib -> core` cycle (core currently owns the proto→arrow schema converter the contrib needs, which is why the thin Delta shim lives in core rather than in the contrib crate).
### Priority
Low. Unlike the JVM side, this coupling is compile-time and feature-gated: a default build links **zero** contrib symbols, which `dev/verify-contrib-delta-gate.sh` asserts in CI. So this is source-level tidiness, not a cost paid by shipped default artifacts.
---
🤖 Filed with [Claude Code](https://claude.com/claude-code).
Contributor guide
Research direction
Start with the current OpStruct::ContribScan dispatcher described in the issue, then read the review thread on #4952 and the two proposed extension paths. Check dev/verify-contrib-delta-gate.sh to understand the default-build constraint. Done means removing per-contrib arms while preserving generic dispatch and the zero-contrib-symbol default build.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100