[SPARK-58394][SQL] Use ZipFile instead of ZipArchiveInputStream for zip archives - #57588
Open
akshatshenoi-db wants to merge 2 commits into
Open
[SPARK-58394][SQL] Use ZipFile instead of ZipArchiveInputStream for zip archives#57588akshatshenoi-db wants to merge 2 commits into
akshatshenoi-db wants to merge 2 commits into
Conversation
…ip archives commons-compress recommends preferring the random-access `ZipFile` over the streaming `ZipArchiveInputStream`: the streaming reader parses local file headers sequentially (so it can return entries absent from the central directory, duplicate names, and incomplete metadata) and cannot handle STORED entries sized only by a trailing data descriptor. `ZipFile` reads the central directory first, giving correct metadata and reliable extraction across all compression methods. This reuses the seekable-channel abstraction already built for 7z (`HadoopSeekableByteChannel`): `openArchiveStream` now returns an `Iterator[(ArchiveEntry, InputStream)]`, letting each container expose its natural shape -- tar/7z advance one forward cursor; zip serves each entry's own `getInputStream` from the central directory. `ZipFile#canReadEntryData` flags encrypted / unsupported-method entries with a clear `CANNOT_READ_ZIP_ENTRY` error. The tar and 7z paths are unchanged behaviorally. ### Why are the changes needed? Correctly reads STORED-with-data-descriptor zips and other zips the streaming reader rejects, and yields correct per-entry metadata (a prerequisite for future zip parallelization via per-entry central-directory offsets). ### Does this PR introduce any user-facing change? Yes, gated by `spark.sql.files.archive.reader.enabled` (default false): zip archives that previously failed to read (e.g. STORED with a data descriptor) now read successfully. ### How was this patch tested? Updated `SupportsArchiveFormatSuite`: the previously-non-streamable STORED+data-descriptor zip now reads successfully; added duplicate-name, encrypted-entry (`CANNOT_READ_ZIP_ENTRY`), and corrupt-archive cases. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code
…ding Self-review fixes: TaskContextImpl takes cpuAmount (not the DBR-only cpus) in OSS, and the ArchiveEntries scaladoc now says (entry, stream) to match the type.
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 changes were proposed in this pull request?
commons-compress recommends preferring the random-access
ZipFileover the streamingZipArchiveInputStream: the streaming reader parses local file headers sequentially (so it can return entries absent from the central directory, duplicate names, and incomplete metadata) and cannot handle STORED entries sized only by a trailing data descriptor.ZipFilereads the central directory first, giving correct metadata and reliable extraction across all compression methods.This reuses the seekable-channel abstraction already built for 7z (
HadoopSeekableByteChannel):openArchiveStreamnow returns anIterator[(ArchiveEntry, InputStream)], letting each container expose its natural shape -- tar/7z advance one forward cursor, while zip serves each entry's owngetInputStreamfrom the central directory.ZipFile#canReadEntryDataflags encrypted / unsupported-method entries with a clearCANNOT_READ_ZIP_ENTRYerror. The tar and 7z paths are unchanged behaviorally.This is part of the archive-reader series (SPARK-57135 CSV, SPARK-57321 CSV-infer, SPARK-57419 JSON, SPARK-57478 text, SPARK-57479 XML, SPARK-57481 Avro, SPARK-57705 zip container).
Why are the changes needed?
The streaming reader rejects valid zips -- notably STORED entries whose size lives only in a trailing data descriptor -- and exposes incomplete/duplicated entry metadata. Reading the central directory fixes both, and the per-entry byte offsets it exposes are a prerequisite for future zip parallelization.
Does this PR introduce any user-facing change?
Yes, gated by
spark.sql.files.archive.reader.enabled(default false): zip archives that previously failed to read (e.g. a STORED entry sized by a trailing data descriptor) now read successfully.How was this patch tested?
Updated
SupportsArchiveFormatSuite: the previously-non-streamable STORED+data-descriptor zip now reads successfully (assertion flipped); added duplicate-name, encrypted-entry (assertingCANNOT_READ_ZIP_ENTRYviacheckError), and corrupt-archive cases. The existing tar and 7z cases are unchanged.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code