apache / apache/datafusion-comet

Make native contrib scan dispatch generic (remove per-contrib arms from core Rust)

Open
#5,379 0 comments 0 reactions 0 assignees View on GitHub
area:scan enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.