Add optional native Lance scan support#4633
Conversation
|
Thanks @wirybeaver. I plan on reviewing this next week. |
|
@wirybeaver thank you for this contribution and sorry for not getting to this sooner. The general direction we are now recommending for Comet data sources is to add them in a contrib directory until they are mature and/or have regular maintainers who can maintain them. — below are the concrete changes to adopt the SPI, significantly shrinking the core footprint. 1. Delete
|
| File | What stays |
|---|---|
CometConf.scala |
COMET_LANCE_NATIVE_ENABLED config entry |
operator.proto |
LanceScan, LanceScanCommon, LanceScanPartition messages + lance_scan = 118 |
planner.rs |
OpStruct::LanceScan arm with #[cfg(feature = "contrib-lance")] gate |
operators/lance_scan.rs |
Rust LanceScanExec behind #[cfg(feature = "contrib-lance")] |
operators/mod.rs |
Feature-gated mod lance_scan + pub use |
operator_registry.rs |
LanceScan variant in the enum |
jni_api.rs |
OpStruct::LanceScan(_) => "LanceScan" name |
pom.xml / spark/pom.xml |
contrib-lance profile + source dir wiring |
Everything else (LanceIntegration.scala, CometLanceNativeScanLike.scala, LancePlanDataInjector, the operators.scala modifications to foreachUntilCometInput / findAllPlanData, the Lance case in CometScanRule) moves to spark/src/contrib-lance/.
5. MSRV bump
The PR bumps rust-version from 1.88 to 1.92 for the entire workspace. If the Lance crate requires 1.92, gate it behind the feature flag or pin a lance rev that compiles on 1.88.
The net/desired result: a default build (no -Pcontrib-lance) that sees only the config entry and proto message on the Scala side, plus feature-gated-dead Rust code — identical to the Delta SPI pattern.
Which issue does this PR close?
Closes #4632.
Rationale for this change
Comet already has a native table-scan path for Iceberg. Lance tables are currently planned and read through Lance Spark. This prototype keeps Lance Spark as the Spark planning contract, then lets an optional Comet contrib reader detect Lance V2 scans, extract a stable descriptor from Lance Spark, and execute the assigned Lance fragments through native Rust Lance APIs.
The Lance Spark side of the descriptor contract is proposed in lance-format/lance-spark#624.
What changes are included in this PR?
contrib-lanceMaven profile and Rustcontrib-lancefeature.spark.comet.scan.lanceNative.enabled, disabled by default.BatchScanExecplans and delegate to contrib-lance when present and enabled.lance_scan = 118and split-mode payloads.LanceScanExecusing the Rust Lance API for dataset open, fragment selection, projection, filter SQL, limit/offset, batch size, and record batch streaming.This is intentionally a draft prototype. Minimal v1 scope is ordinary Lance table reads only. Index/search reads, namespace-backed credential refresh, metadata/version columns, aggregation pushdown, and production CI coverage are future phases.
Known blocker before this can be merge-ready: packaged Comet currently contains
org.apache.arrow.cclasses rewritten against Comet's shaded Arrow allocator, while Lance Spark expects the normal Arrow C Data ABI. A packaged Spark smoke with both jars exposes this classpath conflict. We need an explicit Arrow C Data packaging/classloader strategy for Comet + Lance Spark before merging a production-ready native Lance reader.How are these changes tested?
Passed:
~/.cargo/bin/cargo check -p datafusion-comet --no-default-features~/.cargo/bin/cargo check -p datafusion-comet --no-default-features --features contrib-lance./mvnw test -Dtest=none -Dsuites="org.apache.comet.rules.CometScanRuleSuite" -Pspark-4.1,contrib-lance -Dscalastyle.skip=true./mvnw package -DskipTests -Pspark-4.1,contrib-lance -Dscalastyle.skip=trueSmoke attempted:
source ~/uvenv/common/bin/activate && python /home/user/draft/comet_lance_native_smoke.pyThe smoke writes and reads a local Lance dataset, but packaged Comet + Lance Spark currently fails at runtime with an Arrow C Data ABI/classpath conflict as described above. The draft PR keeps that blocker visible for design review instead of hiding it behind unit-only coverage.