Skip to content

Add optional native Lance scan support#4633

Draft
wirybeaver wants to merge 7 commits into
apache:mainfrom
wirybeaver:xuanyili/lance
Draft

Add optional native Lance scan support#4633
wirybeaver wants to merge 7 commits into
apache:mainfrom
wirybeaver:xuanyili/lance

Conversation

@wirybeaver

@wirybeaver wirybeaver commented Jun 12, 2026

Copy link
Copy Markdown

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:

  • Adds a generic CometScanContrib hook so optional contrib modules can participate in V2 scan planning without putting format-specific logic in core scan rules.
  • Adds a build-gated contrib-lance Maven profile and Rust contrib-lance feature, plus spark.comet.scan.lanceNative.enabled=false as the runtime gate.
  • Adds Lance-specific Scala contrib code that reflectively detects Lance Spark LanceScan, calls the proposed nativeScanPlan() descriptor, serializes it to a typed lance_scan = 118 proto payload, and injects per-partition split data through CometScanWithPlanData.
  • Adds native Rust LanceScanExec that 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.
  • Keeps default Comet builds dependency-free with respect to Lance Spark and falls back to the original Spark BatchScanExec when 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.c classes 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=true
  • cargo check -p datafusion-comet --no-default-features
  • cargo check -p datafusion-comet --no-default-features --features contrib-lance
  • git diff --check

Native 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 Spark BatchScanExec.

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:

source ~/uvenv/common/bin/activate

cd ~/lance-spark
mvn -DskipTests package

cd ~/datafusion-comet
./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=true

cd native
cargo check -p datafusion-comet --no-default-features --features contrib-lance

The local Spark smoke should use the locally built Lance Spark bundle, for example:

/home/user/lance-spark/lance-spark-bundle-4.1_2.13/target/lance-spark-bundle-4.1_2.13-0.5.1.jar

A packaged Comet + local Lance Spark smoke was attempted through:

  • source ~/uvenv/common/bin/activate && python /home/user/draft/comet_lance_native_smoke.py

That smoke writes and reads a local Lance dataset, but currently fails when both packaged jars are present because Comet packages org.apache.arrow.c classes 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.

@andygrove

Copy link
Copy Markdown
Member

Thanks @wirybeaver. I plan on reviewing this next week.

@parthchandra

Copy link
Copy Markdown
Contributor

@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.
To that end, @schenksj 's PR #4700 merged an SPI (CometScanWithPlanData trait + ServiceLoader-based PlanDataInjector discovery + generalized foreachUntilCometInput / findAllPlanData) specifically so that contribs like this one don't need to touch core. This PR predates that merge so it still wires Lance in the old way

— below are the concrete changes to adopt the SPI, significantly shrinking the core footprint.


1. Delete CometLanceNativeScanLike — use CometScanWithPlanData instead

CometLanceNativeScanLike duplicates what the now-merged CometScanWithPlanData trait already provides (sourceKey, commonData, perPartitionData, plus optional dynamicPruningFilters / withDynamicPruningFilters).

In CometLanceNativeScanExec (contrib), change:

// Before
extends CometLeafExec with CometLanceNativeScanLike

// After
extends CometLeafExec with CometScanWithPlanData

Delete CometLanceNativeScanLike.scala from core entirely.

This single change makes the operators.scala modifications unnecessary:

  • foreachUntilCometInput now matches case _: CometLeafExec as its first arm — any CometLeafExec is recognized as an input boundary so no enumeration entry is needed.
  • findAllPlanData has a generic arm case s: CometLeafExec with CometScanWithPlanData => ... that calls ensureSubqueriesResolved() and collects data keyed by sourceKey and no Lance-specific case is needed.

Delete both the _: CometLanceNativeScanLike and case lance: CometLanceNativeScanLike additions from operators.scala.


2. Move LancePlanDataInjector to contrib via ServiceLoader

The merged SPI discovers contrib PlanDataInjectors via java.util.ServiceLoader. Instead of adding LancePlanDataInjector to the hardcoded injectors list in core:

  1. Move LancePlanDataInjector into spark/src/contrib-lance/scala/... (make it a class, not an object, so ServiceLoader can instantiate it via no-arg ctor).
  2. Add a service file at spark/src/contrib-lance/resources/META-INF/services/org.apache.spark.sql.comet.PlanDataInjector containing:
    org.apache.comet.lance.LancePlanDataInjector
  3. Remove the LancePlanDataInjector definition and registry entry from operators.scala.

3. Move LanceIntegration + CometScanRule hook to contrib

spark/src/main/scala/org/apache/comet/lance/LanceIntegration.scala is a reflection bridge in core. The pattern established by the Delta SPI is: core does not reference contrib, not even reflectively.

Recommended approach:

  • Keep COMET_LANCE_NATIVE_ENABLED in CometConf.scala (config entries in core are fine — Iceberg does this too).
  • Move scan detection (isLanceScan, nativeScanPlan, tryCreateNativeScan) into contrib, e.g. org.apache.comet.lance.LanceScanRuleExtension.
  • For the CometScanRule hook: define a tiny trait CometScanContrib { def tryTransform(scanExec: BatchScanExec): Option[SparkPlan] } in core, discover implementations via ServiceLoader in CometScanRule at the case scanExec: BatchScanExec => match before the Iceberg arm, and let contrib
    provide the implementation. This way core gets a ~3-line generic dispatch, not a Lance-specific case.

Delete LanceIntegration.scala from core and the LanceIntegration.isLanceScan / tryCreateNativeScan case from CometScanRule.scala.


4. What remains in core after these changes

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.


@wirybeaver

Copy link
Copy Markdown
Author

@parthchandra Thanks for your point out. I will move the Lance comet connector to the contrib directory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add optional native Lance scan support

3 participants