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.
|
@parthchandra Thanks for your point out. I will move the Lance comet connector to the contrib directory |
2debee5 to
f062efa
Compare
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?
At a high level, this PR adds the Comet side of an experimental, opt-in native Lance read path:
CometScanContribhook so optional contrib modules can participate in V2 scan planning without putting format-specific logic in core scan rules.contrib-lanceMaven profile and Rustcontrib-lancefeature, plusspark.comet.scan.lanceNative.enabled=falseas the runtime gate.LanceScan, calls the proposednativeScanPlan()descriptor, serializes it to a typedlance_scan = 118proto payload, and injects per-partition split data throughCometScanWithPlanData.LanceScanExecthat opens the Lance dataset, pins the resolved version, applies storage options/projection/filter/limit/batch size, selects the assigned fragments, and streams Arrow batches back into Comet.BatchScanExecwhen the profile, config, descriptor, schema, or storage mode is unsupported.Please review the ASCII architecture diagram in #4632 first. It shows how the pieces fit together across Spark/lance-spark planning, Comet core extension points,
contrib-lance, the native proto payload, and Rust Lance execution.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?
Unit/build checks that do not require a released Lance Spark descriptor:
./mvnw test -Dtest=none -Dsuites="org.apache.spark.sql.comet.PlanDataInjectorSuite,org.apache.spark.sql.comet.CometScanWithPlanDataSuite" -Pspark-4.1 -Dscalastyle.skip=true./mvnw test -Dtest=none -Dsuites="org.apache.comet.rules.CometScanRuleSuite,org.apache.spark.sql.comet.PlanDataInjectorSuite,org.apache.spark.sql.comet.CometScanWithPlanDataSuite" -Pspark-4.1,contrib-lance -Dscalastyle.skip=truecargo check -p datafusion-comet --no-default-featurescargo check -p datafusion-comet --no-default-features --features contrib-lancegit diff --checkNative activation cannot be tested with the official Lance Spark artifact yet because no released Lance Spark jar has
org.lance.spark.read.LanceScan.nativeScanPlan(). With the official artifact, Comet should only exercise the reflective fallback path and keep the original SparkBatchScanExec.For local end-to-end smoke testing, build the matching Lance Spark PR branch and put that local bundle jar on Spark's runtime classpath:
The local Spark smoke should use the locally built Lance Spark bundle, for example:
A packaged Comet + local Lance Spark smoke was attempted through:
source ~/uvenv/common/bin/activate && python /home/user/draft/comet_lance_native_smoke.pyThat smoke writes and reads a local Lance dataset, but currently fails when both packaged jars are present because Comet packages
org.apache.arrow.cclasses rewritten against Comet's shaded Arrow allocator while Lance Spark expects the normal Arrow C Data ABI. This PR intentionally keeps that packaging/classpath blocker visible for design review.