ErikBPF opened a new pull request, #6945: URL: https://github.com/apache/datafusion-comet/pull/6945
## Which issue does this PR close? Closes #5656. This draft awaits maintainer review and current-head hosted CI. Please approve current-head CI and enable `run-delta-tests` for a hosted integration verdict. Label-only runs are advisory and do not replace required checks. Reintroduces the accepted candidate from dwsmith1983/datafusion-comet#3 on Apache main after #5365 landed. Deleting the fork's base branch closed the earlier PR. ## Rationale for this change The native-format deletion-vector decoder deserializes compressed `RoaringBitmap` containers, then enumerates every deleted row and reinserts it into a `RoaringTreemap`. This makes decoding scale with deletion cardinality and expands compact runs. On tested upstream base `474b5ece841a7fc6dff37ae04bd6787e654d7be7` with Roaring 0.11.5, the compression regression fails because a one-million-row contiguous sub-bitmap grows from 230 to 131,208 serialized bytes. ## What changes are included in this PR? - Move non-empty decoded sub-bitmaps directly into `RoaringTreemap::from_bitmaps`. - Keep empty implicit partitions omitted without changing subsequent partition keys. - Add the dense-container compression regression and cover an empty intermediate partition in native-format decoding. The change is limited to `native/core/src/execution/delta_dv.rs`. Portable decoding, malformed-input checks, cardinality validation, split-aware access-plan construction, logarithmic skipped-prefix seeking, and memory-reservation accounting remain unchanged. The separate compressed-range candidate for #5655 is not included. This removes only the decoder's per-row reinsertion pass. Access-plan construction still iterates deleted rows to build row-group selections, including partially deleted row groups. ## How are these changes tested? Local validation used upstream base `474b5ece841a7fc6dff37ae04bd6787e654d7be7`, Rust 1.98.1 and Roaring 0.11.5 on Linux. Initial checks used Java 21; an independent hash-verified Orion checkout repeated native validation on JDK 17. - RED: the new compression regression fails on unchanged production code with serialized size 131,208 versus expected 230. - GREEN: all 44 native DV tests pass after the fix, including malformed-input, corruption, split/Parquet and memory-accounting coverage. - Full-reactor `./mvnw -Pspark-4.1 compile -DskipTests`: passed. Orion additionally ran the Delta JVM suites below. - `cargo fmt --all -- --check`: passed. - `cargo clippy --locked --all-targets --workspace -- -D warnings`: passed. - `cargo build --locked -p comet-test-udfs`, then `cargo test --locked -p datafusion-comet --lib`: 874 passed, 5 ignored, zero failures. The first run's 11 missing-fixture failures were resolved by building that documented shared-library fixture; no source or assertion workaround was applied. - Orion independently passed the regression, DV and full core suites, fmt, clippy, native build and Spark 4.1 JVM compilation. Its isolated native target and Maven repository prevented cross-worktree artifact reuse. Source hashes matched the reviewed candidate; the packaged Spark 4.1 JNI library matched its native build. - Temporary staging uses O(nonempty partitions) metadata; bitmap payloads are moved, not duplicated. No universal peak-memory claim is made. Orion ran root-reactor `clean install -DskipTests`, then `test -Dtest=none -DwildcardSuites=<explicit Delta suites>` for each `-Pspark-<version>,delta`, without `-pl`. XML reports and explicit command exit statuses were checked: | Local integration scope | Passed | Canceled | Failed | | --- | ---: | ---: | ---: | | Spark 3.5: Delta scan, native scan, DML, plan injector | 277 | 1 | 0 | | Spark 4.0: same four suites | 278 | 0 | 0 | | Spark 4.1: same four suites | 278 | 0 | 0 | | Spark 3.5: required MinIO/S3 suite | 3 | 0 | 0 | The canceled Spark 3.5 timestamp case explicitly requires Spark 4+ and passes on both 4.x profiles. `COMET_DELTA_S3_REQUIRED=1` prevented a false green from unavailable containers; all three S3 tests executed through rootless Podman. These are local integration results, not hosted CI approval or complete Spark SQL-suite coverage. ### Decoder measurements An isolated release-mode harness extracts the exact decoder function bodies from that tested upstream base and this patch with Roaring 0.11.5. It supplies a minimal error-type adapter; successful measured payloads never construct errors. Both paths produce identical rows and cardinalities before timing. Each pattern uses one native sub-bitmap spanning one million positions; the contiguous fixture uses `insert_range`, matching the compression regression. | Pattern | Deleted rows | Base / patched microseconds per decode | Base / patched allocated bytes | | --- | ---: | ---: | ---: | | Sparse (0.1%) | 1,000 | 40.780 / 2.715 | 11,888 / 3,216 | | Contiguous compressed run | 1,000,000 | 18,319.494 / 0.917 | 657,506 / 1,282 | | Alternating | 500,000 | 10,161.456 / 22.292 | 788,512 / 132,288 | The contiguous input sub-bitmap is 230 serialized bytes. The tested base expands the decoded sub-bitmap to 131,208 bytes; this patch retains 230 bytes. Numbers are medians of seven 20-decode sample means on an unpinned shared Linux host. Result destruction and counting-System-allocator overhead are included; allocated bytes are cumulative requested bytes, not peak memory or RSS. Input generation and serialization are excluded. These are decoder-only measurements, not access-plan construction, object-store/Parquet I/O, Spark end-to-end latency, or hosted CI evidence. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
