feat: core SPI for contrib leaf scans (CometScanWithPlanData)#3
Draft
schenksj wants to merge 1 commit into
Draft
feat: core SPI for contrib leaf scans (CometScanWithPlanData)#3schenksj wants to merge 1 commit into
schenksj wants to merge 1 commit into
Conversation
schenksj
changed the title
schenksj
force-pushed
the
pr/delta-A1-spi
branch
from
c559ef0 to
e071ec8
Compare
Introduce a small extension contract so out-of-tree Comet contrib leaf scans (Delta, and future Hudi/etc.) can participate in native planning without core holding a compile-time reference to them -- mirroring the Iceberg-precedent of keeping the data-source-specific code at the edge. What this adds: - `trait CometScanWithPlanData` (`sourceKey` / `commonData` / `perPartitionData`, plus optional `dynamicPruningFilters` / `withDynamicPruningFilters` for scans whose DPP filters live in a @transient field). `CometNativeScanExec` now mixes it in. - `CometNativeExec.foreachUntilCometInput` matches `case _: CometLeafExec` (a strict superset of the previous fixed scan enumeration -- all built-in leaf scans already extend `CometLeafExec`), so any leaf Comet exec is recognised as an input boundary. - `PlanDataInjector.findAllPlanData` collects per-partition planning data via the trait instead of a hardcoded `CometNativeScanExec` match. - `PlanDataInjector`'s registry gains one reflective `DeltaPlanDataInjector$` slot, appended only when a contrib bundled it (`-Pcontrib-delta`). Default builds get a `ClassNotFoundException` -> `None` and an unchanged injectors list, so there is zero contrib surface at runtime. - `CometPlanAdaptiveDynamicPruningFilters` rewrites AQE DPP filters in place for trait scans whose filters can't survive `makeCopy` (apache#3510). Inert by construction: with no contrib on the classpath this is behavior- preserving (the leaf match is a superset; the trait match catches the same `CometNativeScanExec`; the reflective slot resolves to nothing). Tests: `CometScanWithPlanDataSuite` (trait-contract defaults + reflective-slot graceful absence). Verified `CometJoinSuite` (native scan fusion / DPP) stays green. First unit of the Delta-contrib PR split (tracking: apache#4366).
schenksj
force-pushed
the
pr/delta-A1-spi
branch
from
e071ec8 to
aa0fd12
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this unit is
The first reviewable slice of the Delta-contrib work: a small extension contract that lets out-of-tree Comet contrib leaf scans (Delta now; Hudi/etc. later) take part in native planning without core compile-time-referencing them — the same edge-keeps-the-source-specific-code shape Iceberg already uses.
Changes
trait CometScanWithPlanData—sourceKey/commonData/perPartitionData, plus optionaldynamicPruningFilters/withDynamicPruningFilters(for scans whose DPP filters live in a@transientfield, Epic: CometNativeScan improvements (per-partition serde, cleanup, DPP, AQE DPP, V2 operator) apache/datafusion-comet#3510).CometNativeScanExecmixes it in.foreachUntilCometInputnow matchescase _: CometLeafExec— a strict superset of the previous fixed scan list (every built-in leaf scan already extendsCometLeafExec).PlanDataInjector.findAllPlanDatacollects per-partition planning data via the trait instead of a hardcodedCometNativeScanExecmatch.PlanDataInjectorregistry gains one reflectiveDeltaPlanDataInjector$slot, appended only when a contrib bundled it (-Pcontrib-delta). Default builds getClassNotFoundException → Noneand an unchanged list.CometPlanAdaptiveDynamicPruningFiltersrewrites AQE DPP filters in place for trait scans whose filters can't survivemakeCopy.What it deliberately does NOT do yet
perPartitionFilePathson the trait — that member only feedsFAILED_READ_FILEerror conversion and lands later (unit A.8, after feat: surface native parquet read failures as FAILED_READ_FILE apache/datafusion-comet#4536).main's current O(N)injectorsloop, not theopStructCase/injectorsByKindrefactor in perf: O(1) PlanDataInjector lookup by op kind apache/datafusion-comet#4535. Whichever of {A.1, perf: O(1) PlanDataInjector lookup by op kind apache/datafusion-comet#4535} lands second takes a small mechanical conflict (accepted).DeltaPlanDataInjectorto find yet; this unit is inert.Why it's safe / inert
With no contrib on the classpath: the leaf match is a superset of the old enumeration, the trait match catches the same
CometNativeScanExec, and the reflective slot resolves to nothing. Behavior-preserving on default builds.Verification
CometScanWithPlanDataSuite(new): trait-contract defaults + reflective-slot graceful absence — 2/2.CometJoinSuite(native scan fusion + DPP path): 28/28.