Repository navigation
Support Spark's AtLeastNNonNulls natively #6093
Description
Activity
@hsiang-c I have a native prototype that is faster than codegen dispatch in local benchmarks, but some
na.drop(...).count()cases still favor Spark fallback. Could you share a representative query, schema, threshold, and approximate NULL/NaN rates so I can test your workload?Historical prototype measurements from 2026-09-23; later implementation and combined-candidate evidence are maintained in the delivery plan. The request for a representative original workload remains open.
take
Delivery plan, refreshed 2026-10-07. This supersedes the earlier dependency status, while retaining historical validation at its original commit.
Item Current role Status / next step #6350 Shared native CASE/IF evaluation Merged 2026-09-30. Use main; do not restore the retired standalone IF optimization. #6396 Generic filter-output pruning Merged 2026-10-04. No longer an implementation prerequisite waiting to land. #6397 Independent IF branch-selection and ANSI short-circuit tests Merged 2026-10-05. These tests were never an implementation prerequisite for #6180. #6180 Native AtLeastNNonNulls for DataFrame.na.drop Still Draft. Published head is c344800, merging main c8e6553. The merge conflict is resolved; final-head CI is running. The published refresh c344800 integrates main c8e6553. Its diff against main contains 17 feature/config/test/benchmark/documentation files, with no separate generic filter-projection implementation diff. Upstream CI and fork #34 CI are running. This is not yet a completed final-head CI verdict.
Review follow-ups:
- Splitting filter-output pruning is complete through perf: prune unused filter output columns #6396.
- The SQL-test request concerned the projection regression, not a blanket request to rewrite the AtLeastNNonNulls DataFrame tests. Main now contains operators/filter_projection.sql and a Scala metrics-specific test through perf: prune unused filter output columns #6396.
- The general-counting crossover is configurable through spark.comet.exec.atLeastNNonNulls.smallBatchThreshold, default 64. Batches below the threshold use row counters; batches at or above it use bitmap counters. Any positive integer is accepted. This does not change the fixed 64-bit bitmap word size, the input batch size, or the number of valid values required by na.drop. The any/all-valid paths are unaffected. This implementation exists in the published branch; its explanation and final-head validation still belong in the PR update.
Current CI evidence:
- The published upstream and fork Introduce unified memory manager #34 head is c344800; its CI is running.
- Old head 0d88dd1 had a Clippy failure on deprecated std::f64 constants in numeric.rs. The new merge includes fix: build with Rust 1.99, which deprecates the legacy f64 constants and fetch_update #6507 / 75d7c7a fixing the Rust 1.99 deprecations. That old failed run must not be described as the new head's verdict.
Historical integration evidence:
Fork #42 preserves candidate 4528a2a and its completed CI at https://github.com/rich7420/datafusion-comet/actions/runs/36812535403. Its local tests and benchmarks describe that exact combined candidate. They do not validate published head 0d88dd1 or the new main refresh. Fork #41 is an older integration candidate, not a branch to merge back into the final feature.Remaining sequence:
- Collect the implementation owner's local validation evidence for published c344800 and wait for its current-head CI verdict.
- Check the final diff contains only AtLeastNNonNulls, its threshold config/plumbing, relevant tests, documentation and benchmarks.
- Validate NULL/NaN counting, empty batches, row/bitmap boundaries, configurable thresholds, scalar/dictionary inputs, native execution and Spark short-circuit/error behavior on the final commit.
- Run the appropriate Spark SQL/profile CI on that final commit. Keep build failures separate from runtime parity failures.
- Re-run representative performance controls with the same main baseline: native versus Comet fallback versus Spark, count-only and full-column consumers. Preserve answer/path assertions and timing variance. The issue author's original workload has not been supplied or verified.
- Update feat: support AtLeastNNonNulls natively #6180's body and this plan with the exact final head and its evidence, then request review. Keep historical fork evidence labeled by commit; retire obsolete active candidates only after the refreshed candidate is verified.
What is the problem the feature request solves?
When I enabled Comet in our workload, I found that
AtLeastNNonNullsis not supported yet so I wired it via theCometCodegenDispatch.It turns out that doing so is slower (~ 8% more in total runtime) than falling back to Spark on
AtLeastNNonNulls.Describe the potential solution
Support
AtLeastNNonNullswith native Rust counterpart.Additional context
No response