schenksj opened a new issue, #5379: URL: https://github.com/apache/datafusion-comet/issues/5379
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). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
