mbutrovich opened a new pull request, #5853: URL: https://github.com/apache/datafusion-comet/pull/5853
## Which issue does this PR close? Part of #3756. ## Rationale for this change Comet's native Iceberg scan reads V3 tables (the V3 gate and the encryption/default-value/unsupported-type fallbacks landed in #4991), but did not read V3 deletion vectors. V3 stores positional deletes as `deletion-vector-v1` Puffin blobs instead of Parquet position-delete files, so any V3 table with row-level deletes fell back to Spark. This PR adds native V3 deletion-vector reads. Native execution runs through iceberg-rust, whose deletion-vector read support has since landed upstream (epic apache/iceberg-rust#2792: the blob codec in apache/iceberg-rust#2866, the scan-task coordinates in apache/iceberg-rust#2868, and the loader and apply rules in apache/iceberg-rust#3035). Comet's pinned rev already includes all three, so this PR changes no dependency. ## What changes are included in this PR? Deletion-vector read path: - Carry the deletion-vector coordinates (`referenced_data_file`, `content_offset`, `content_size_in_bytes`) from the JVM through the scan-task protobuf (`IcebergDeleteFile`), the serde (`CometIcebergNativeScan`), and the native planner into iceberg-rust's `FileScanTaskDeleteFile`. - Carry `record_count`. iceberg-rust requires it for a deletion vector, which it checks against the cardinality it decodes from the blob, and rejects a vector without it. - Carry the delete file's format (`"PARQUET"` / `"PUFFIN"`). iceberg-rust selects its deletion-vector reader on `FileScanTaskDeleteFile::file_format` alone, so the format has to reach the native side rather than being inferred there from the presence of a content offset. - Read the new `DeleteFile` accessors through `IcebergReflection`. A missing accessor means an Iceberg version predating deletion vectors, and a null result means this delete file is not a vector; both leave the field unset. Any other reflection failure propagates, because by serde time the plan is committed to the native scan and silently dropping a vector's coordinates would return its deleted rows. Delete-file gate (`CometScanRule`): - Accept Puffin delete files (deletion vectors) in addition to Parquet delete files. A delete format that is neither, or a format that cannot be determined, still falls back to Spark. Delete-file pool: - #4991 added a deduplicated delete-file pool keyed by file path. Deletion vectors for different data files share a single Puffin file and differ only by content offset, so keying on path alone collapsed distinct vectors into the first one seen and dropped deletes. Key the pool on the full delete-file message instead; Parquet delete files (one artifact per path) still dedup as before. - Because those pool entries are now distinct per data file, the shared Puffin path would repeat once per data file. Intern delete-file paths into a new `delete_file_path_pool` on the common message, referenced by index, so the path is serialized once regardless of how many vectors point at it. The new pool is reported in the existing dedup summary and per-pool byte breakdown. ## How are these changes tested? - New end-to-end test `CometIcebergNativeSuite."MOR V3 table with DELETION VECTORS - verify deletes are applied"`: creates a V3 merge-on-read table, deletes rows spread across multiple data files (which Spark writes as deletion vectors packed into a shared Puffin file), asserts a Puffin delete file exists via the `.files` metadata table, then verifies the native scan result matches Spark and runs through `CometIcebergNativeScanExec`. The multi-data-file layout is what exercises the delete-file pool fix above. The test is guarded to Iceberg 1.11+ and skips on older versions that predate V3. - The existing `CometIcebergNativeSuite` delete-file pool test, which asserts no delete-file path appears more than once in the serialized common message, now reads the distinct paths from `delete_file_path_pool` and so covers the path interning directly. - The Apache Iceberg Spark delete-read suites (`TestSparkReaderDeletes`, `TestSparkDistributedDataScanDeletes`) at format version 3 read deletion-vector tables through the native scan (Spark 4.1, via `dev/diffs/iceberg/1.11.0.diff`). - The Iceberg-Spark CI matrix (Spark 3.4, 3.5, and 4.1, against Iceberg 1.8 through 1.11) covers V1/V2 behavior across the supported Iceberg versions. - Correctness of the deletion-vector decode, scan-task propagation, and loader is covered by unit and integration tests in the corresponding iceberg-rust changes. -- 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]
