Skip to content

Add optional native Lance scan support#4633

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

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

Conversation

@wirybeaver

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?

  • Adds an opt-in contrib-lance Maven profile and Rust contrib-lance feature.
  • Adds a small reflection-only Lance bridge in Comet core so default builds do not depend on Lance Spark.
  • Adds spark.comet.scan.lanceNative.enabled, disabled by default.
  • Extends scan planning to detect Lance BatchScanExec plans and delegate to contrib-lance when present and enabled.
  • Adds typed native proto support with lance_scan = 118 and split-mode payloads.
  • Adds Scala contrib serialization/execution classes for Lance native scans.
  • Adds Rust native LanceScanExec using 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.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?

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=true

Smoke attempted:

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

The 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.

@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.


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