[SPARK-58605][SQL] Verify parent-archive _metadata for archive reads - #57805
Open
akshatshenoi-db wants to merge 1 commit into
Open
[SPARK-58605][SQL] Verify parent-archive _metadata for archive reads#57805akshatshenoi-db wants to merge 1 commit into
akshatshenoi-db wants to merge 1 commit into
Conversation
### What changes were proposed in this pull request? Adds tests asserting that reading an archive exposes the parent archive file's values in the `_metadata` column (`file_path`, `file_name`, `file_size`, `file_modification_time`, `file_block_start`, `file_block_length`). An archive is read as a single non-splittable `PartitionedFile`, so the generic `FileFormat` metadata extractors already produce the parent archive's values (`file_block_start` = 0, `file_block_length` = `file_size` = the archive's size). This is test-only: no production change is needed, and the tests pin the behavior so it cannot silently regress. Inner-file `_metadata` is explicitly a non-goal: because the archive is a single split, the `_metadata` mechanism cannot attribute a row to its originating inner entry. ### Why are the changes needed? The parent-archive `_metadata` contract was untested, so a future change to splitting or to the metadata extractors could regress it unnoticed. ### Does this PR introduce any user-facing change? No. Test-only; archive reading remains gated by `spark.sql.files.archive.reader.enabled` (default false). ### How was this patch tested? New shared test in `ArchiveReadSuiteBase`, which runs for every format x container suite (csv/json/xml/avro x tar/zip/7z), plus the parallel test in `TextArchiveReadBase` and `BinaryFileArchiveReadBase` (which do not extend `ArchiveReadSuiteBase`). For binaryFile with `wholeFile=false` the test also pins that `_metadata` stays parent-only even though the `path`/`length` data columns are per entry. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code
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?
Adds tests asserting that reading an archive exposes the parent archive file's values in the
_metadatacolumn (file_path,file_name,file_size,file_modification_time,file_block_start,file_block_length).An archive is read as a single non-splittable
PartitionedFile, so the genericFileFormatmetadata extractors already produce the parent archive's values (file_block_start= 0,file_block_length=file_size= the archive's size on disk). This PR is therefore test-only: no production change was needed. The tests pin the behavior so it cannot silently regress.Inner-file
_metadatais explicitly a non-goal: because the archive is a single split, the_metadatamechanism operates atPartitionedFilegranularity and cannot attribute a row to its originating inner entry.This follows the archive-reader series: SPARK-57135 / SPARK-57321 (CSV), SPARK-57419 (JSON), SPARK-57478 (text), SPARK-57479 (XML), SPARK-57481 (Avro), SPARK-58382 (binaryFile), SPARK-57705 (zip), SPARK-58246 (7z).
Why are the changes needed?
The parent-archive
_metadatacontract was entirely untested. A future change to file splitting or to the metadata extractors could regress it unnoticed, and users reading archives rely on_metadatato identify the source archive file.Does this PR introduce any user-facing change?
No. Test-only; archive reading remains gated by
spark.sql.files.archive.reader.enabled(default false).How was this patch tested?
A new shared test in
ArchiveReadSuiteBase, which runs for every format x container suite (csv/json/xml/avro x tar/zip/7z), asserting every row of a multi-entry archive carries the same parent-archive values, withfile_block_start = 0andfile_block_length = file_size =the archive's size.Plus the parallel test in
TextArchiveReadBaseandBinaryFileArchiveReadBase, which do not extendArchiveReadSuiteBase(their row shapes differ). For binaryFile withwholeFile=falsethe test additionally pins that_metadatastays parent-only even though thepath/lengthdata columns are sourced per entry.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code