[SPARK-58609][SQL] Add archivePathFilter option to select inner archive entries - #57808
[SPARK-58609][SQL] Add archivePathFilter option to select inner archive entries#57808akshatshenoi-db wants to merge 6 commits into
Conversation
…ve entries Adds a new file-source option `archivePathFilter`, a glob that selects which inner entries of an archive are ingested, matched against each entry's full path within the archive so patterns like `subdir/*` and `*/*.csv` work. `pathGlobFilter` cannot express this: it must already match the archive file for the file to be listed, so it cannot simultaneously select a subset of the entries inside the archive. The glob is applied in addition to `ignoredPathSegmentRegex` (both must pass), and is inert when archive reading is disabled. `FileSourceOptions` carries the glob string plus `archivePathFilterPattern`, a transient lazy val holding the compiled Hadoop `GlobPattern`, so the glob is compiled once per executor JVM rather than once per archive. The glob string is validated on the driver so an invalid glob fails fast. `SupportsArchiveFormat.readArchiveEntries` and the random-access `readLocalizedEntries` / `localizeEntries` take an optional filter (default `None`, so existing callers are unaffected) applied in `shouldSkipEntry`. It is threaded from the read paths of CSV, JSON, text, XML, Avro, binaryFile, Parquet and ORC.
…ply it to inference - An empty `archivePathFilter` was retained and compiled to a `GlobPattern` that matches only the empty string, so every archive entry was skipped and the scan returned zero rows. Treat an empty value as absent, matching how an empty `ignoredPathSegmentRegex` disables that filter. - Remove the `None` default from the `archivePathFilter` parameter of `readArchiveEntries`, `readLocalizedEntries` and `localizeEntries`, so no call site can silently skip it. - Apply the filter to schema inference, which previously read every entry while the scan honored the filter. The inferred schema was therefore computed over a superset of the scanned entries. Threaded through CSV/JSON/XML (capturing the glob string, since the compiled `GlobPattern` is not serializable), Avro, ORC and Parquet. - Add a shared `ArchiveReadSuiteBase` case that puts an extra column in a filtered-out entry and asserts the inferred schema excludes it, so it runs for every format and container.
cloud-fan
left a comment
There was a problem hiding this comment.
0 blocking, 1 non-blocking, 1 nit.
The filtering behavior and cross-format propagation are coherent, with two non-blocking cleanup items around matcher cache scope and repeated inference-time compilation.
Nits: 1 minor item (see inline comments).
Suggestions (1)
- sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVDataSource.scala:185: This compilation runs once per archive in the flatMap callback, so partitions containing multiple archives repeatedly compile the same glob; JSON and XML inference have the same shape. Please compile once in mapPartitions and reuse the matcher across that partition's archive streams. -- see inline
Verification
I traced the option from FileSourceOptions through the shared archive-entry predicate and through scan and schema-inference paths for the supported formats. The filter is applied before parsing and composes with the existing per-segment hidden-path check; Parquet and ORC transport the string through Hadoop configuration for parallel schema work, while CSV, JSON, and XML currently compile it inside each archive callback.
PR metadata suggestions
- Correct the claim that archivePathFilterPattern is compiled once per executor JVM; it is cached per FileSourceOptions instance, and CSV/JSON/XML inference currently compiles once per archive.
…time compilation - Correct the `archivePathFilterPattern` scaladoc. A `lazy val` caches per instance of the class, not per JVM, and the schema-inference RDDs and the parallel footer/schema readers cannot reach that value at all, so they carry the glob string and compile it themselves. Describe both accurately. - CSV, JSON and XML inference compiled the glob inside the `flatMap` callback, so a partition holding several archives recompiled the same glob for each one. Compile once per partition with `mapPartitions` and reuse the matcher. The per-element `skipInputOnError` wrapping stays per element so a single corrupt input is still skipped individually rather than failing the partition. - Add the binaryFile `archivePathFilter` case to `BinaryFileArchiveReadBase`, which had production wiring but no test coverage for the filter.
cloud-fan
left a comment
There was a problem hiding this comment.
2 addressed, 0 remaining, 3 new. (3 newly introduced, 0 late catches, 0 previously raised.)
0 blocking, 2 non-blocking, 1 nit.
The filtering behavior is coherent across formats and the previous review concerns are addressed; three small cleanup items remain.
Design / architecture (1)
- .isaac/config.json:2: Remove
.isaac/config.json; it is unrelated local tool state. -- see inline
Suggestions (1)
- sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVDataSource.scala:180: Initialize the inference matcher lazily so ordinary-file partitions do not compile an unused glob. -- see inline
Nits: 1 minor item (see inline comments).
Verification
I traced the option from parsing and validation through the shared entry predicate, streaming and random-access scans, and each schema-inference transport path. The shared tests cover the user-visible selection contract and its composition with existing hidden-entry filtering.
… stray file - Remove `.isaac/config.json`, which was local tooling state committed by mistake and is unrelated to this change. - Make the per-partition matcher in the CSV, JSON and XML inference paths a `lazy val`. The eager `val` compiled the glob as soon as the partition function ran, so a partition holding only ordinary files paid for a compile it never used; lazy keeps the once-per-partition reuse and compiles only if the archive branch is reached. - Reword the `archivePathFilterPattern` scaladoc as suggested.
cloud-fan
left a comment
There was a problem hiding this comment.
3 addressed, 0 remaining, 0 new.
0 blocking, 0 non-blocking, 0 nits.
The three concerns from the prior review are addressed, and the current implementation is coherent across scan and inference paths.
Verification
I traced option parsing and validation through the shared archive-entry predicate, streaming and localized readers, and each format-specific schema-inference transport path. The latest changes make the documented cache scope precise and use partition-local lazy matcher compilation for CSV, JSON, and XML inference.
PR metadata suggestions
- Correct the claim that the SupportsArchiveFormat helper parameters default to None; the current helper signatures require callers to pass the optional matcher explicitly.
|
@akshatshenoi-db can you make the CI green? |
…ve entries ### What changes were proposed in this pull request? This PR adds a new file-source option `archivePathFilter`: a glob that selects which **inner entries** of an archive are ingested, matched against each entry's full path within the archive, so patterns like `subdir/*` and `*/*.csv` work. It continues the archive-read series (SPARK-57135 / SPARK-57321 CSV, SPARK-57419 JSON, SPARK-57478 text, SPARK-57479 XML, SPARK-57481 Avro, SPARK-57590 Parquet, SPARK-57591 ORC, SPARK-58382 binaryFile, SPARK-58110 trait extraction). Details: - `FileSourceOptions` gains the `archivePathFilter` option (the glob string) plus `archivePathFilterPattern`, a `transient lazy val` holding the compiled Hadoop `GlobPattern`. That caches per `FileSourceOptions` instance, so the reads sharing one options object reuse a single matcher instead of recompiling per archive; it is `transient` because `GlobPattern` is not serializable, so the string travels and an executor recompiles on first use. Two kinds of call site cannot reach that value and carry the glob string instead: the schema-inference RDDs, which would otherwise have to capture the matcher in a closure, compile once per partition via `mapPartitions`; and the parallel footer/schema readers for Parquet and ORC, whose signatures are fixed by `SchemaMergeUtils`, read the glob from the Hadoop configuration. The glob string is validated on the driver, so an invalid glob raises a clear `IllegalArgumentException` naming the option. - `SupportsArchiveFormat.readArchiveEntries` and the random-access `readLocalizedEntries` / `localizeEntries` take an optional filter (default `None`, so existing callers are unaffected), applied in `shouldSkipEntry`. - The option is threaded from the read paths of CSV, JSON, text, XML, Avro, binaryFile, Parquet and ORC. The filter is applied **in addition to** `ignoredPathSegmentRegex` (both must pass), so hidden entries stay hidden even when they match the glob. ### Why are the changes needed? `pathGlobFilter` cannot express this. It is applied during file listing, so it must already match the archive file itself for that archive to be read at all; it cannot simultaneously select a subset of the entries *inside* the archive. Without `archivePathFilter` there is no way to read only part of an archive, so a user wanting a few entries has to read and discard the rest. ### Does this PR introduce _any_ user-facing change? Yes. A new read option `archivePathFilter` (a glob string, no default) is available to the file sources that support archive reads. When set, only archive entries whose full inner path matches the glob are ingested. Reads that do not set it are unaffected, and the whole archive-read feature remains gated by `spark.sql.files.archive.reader.enabled` (default `false`). ### How was this patch tested? New shared cases in `ArchiveReadSuiteBase`, which run for every (format, container) pair -- CSV, JSON, XML, text, Avro, Parquet, ORC and binaryFile across tar, zip and 7z: - selection by inner path (`sub/*`), - an extension glob matching across subdirectories, including an entry that does not match the glob, - a glob matching no entry yields zero rows, - composition with `ignoredPathSegmentRegex` (a hidden entry matching the glob is still skipped). New unit cases in `SupportsArchiveFormatSuite` cover the entry-skip logic directly (glob matching the full path, `*` crossing `/`, no-match, interaction with hidden-entry filtering) and invalid-glob rejection. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code Closes #57808 from akshatshenoi-db/archive-pathfilter. Lead-authored-by: akshatshenoi-db <akshat.shenoi@databricks.com> Co-authored-by: Wenchen Fan <cloud0fan@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 7d5a454) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
…ve entries ### What changes were proposed in this pull request? This PR adds a new file-source option `archivePathFilter`: a glob that selects which **inner entries** of an archive are ingested, matched against each entry's full path within the archive, so patterns like `subdir/*` and `*/*.csv` work. It continues the archive-read series (SPARK-57135 / SPARK-57321 CSV, SPARK-57419 JSON, SPARK-57478 text, SPARK-57479 XML, SPARK-57481 Avro, SPARK-57590 Parquet, SPARK-57591 ORC, SPARK-58382 binaryFile, SPARK-58110 trait extraction). Details: - `FileSourceOptions` gains the `archivePathFilter` option (the glob string) plus `archivePathFilterPattern`, a `transient lazy val` holding the compiled Hadoop `GlobPattern`. That caches per `FileSourceOptions` instance, so the reads sharing one options object reuse a single matcher instead of recompiling per archive; it is `transient` because `GlobPattern` is not serializable, so the string travels and an executor recompiles on first use. Two kinds of call site cannot reach that value and carry the glob string instead: the schema-inference RDDs, which would otherwise have to capture the matcher in a closure, compile once per partition via `mapPartitions`; and the parallel footer/schema readers for Parquet and ORC, whose signatures are fixed by `SchemaMergeUtils`, read the glob from the Hadoop configuration. The glob string is validated on the driver, so an invalid glob raises a clear `IllegalArgumentException` naming the option. - `SupportsArchiveFormat.readArchiveEntries` and the random-access `readLocalizedEntries` / `localizeEntries` take an optional filter (default `None`, so existing callers are unaffected), applied in `shouldSkipEntry`. - The option is threaded from the read paths of CSV, JSON, text, XML, Avro, binaryFile, Parquet and ORC. The filter is applied **in addition to** `ignoredPathSegmentRegex` (both must pass), so hidden entries stay hidden even when they match the glob. ### Why are the changes needed? `pathGlobFilter` cannot express this. It is applied during file listing, so it must already match the archive file itself for that archive to be read at all; it cannot simultaneously select a subset of the entries *inside* the archive. Without `archivePathFilter` there is no way to read only part of an archive, so a user wanting a few entries has to read and discard the rest. ### Does this PR introduce _any_ user-facing change? Yes. A new read option `archivePathFilter` (a glob string, no default) is available to the file sources that support archive reads. When set, only archive entries whose full inner path matches the glob are ingested. Reads that do not set it are unaffected, and the whole archive-read feature remains gated by `spark.sql.files.archive.reader.enabled` (default `false`). ### How was this patch tested? New shared cases in `ArchiveReadSuiteBase`, which run for every (format, container) pair -- CSV, JSON, XML, text, Avro, Parquet, ORC and binaryFile across tar, zip and 7z: - selection by inner path (`sub/*`), - an extension glob matching across subdirectories, including an entry that does not match the glob, - a glob matching no entry yields zero rows, - composition with `ignoredPathSegmentRegex` (a hidden entry matching the glob is still skipped). New unit cases in `SupportsArchiveFormatSuite` cover the entry-skip logic directly (glob matching the full path, `*` crossing `/`, no-match, interaction with hidden-entry filtering) and invalid-glob rejection. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code Closes #57808 from akshatshenoi-db/archive-pathfilter. Lead-authored-by: akshatshenoi-db <akshat.shenoi@databricks.com> Co-authored-by: Wenchen Fan <cloud0fan@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 7d5a454) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
What changes were proposed in this pull request?
This PR adds a new file-source option
archivePathFilter: a glob that selects which inner entries of an archive are ingested, matched against each entry's full path within the archive, so patterns likesubdir/*and*/*.csvwork.It continues the archive-read series (SPARK-57135 / SPARK-57321 CSV, SPARK-57419 JSON, SPARK-57478 text, SPARK-57479 XML, SPARK-57481 Avro, SPARK-57590 Parquet, SPARK-57591 ORC, SPARK-58382 binaryFile, SPARK-58110 trait extraction).
Details:
FileSourceOptionsgains thearchivePathFilteroption (the glob string) plusarchivePathFilterPattern, a@transient lazy valholding the compiled HadoopGlobPattern. That caches perFileSourceOptionsinstance, so the reads sharing one options object reuse a single matcher instead of recompiling per archive; it istransientbecauseGlobPatternis not serializable, so the string travels and an executor recompiles on first use. Two kinds of call site cannot reach that value and carry the glob string instead: the schema-inference RDDs, which would otherwise have to capture the matcher in a closure, compile once per partition viamapPartitions; and the parallel footer/schema readers for Parquet and ORC, whose signatures are fixed bySchemaMergeUtils, read the glob from the Hadoop configuration. The glob string is validated on the driver, so an invalid glob raises a clearIllegalArgumentExceptionnaming the option.SupportsArchiveFormat.readArchiveEntriesand the random-accessreadLocalizedEntries/localizeEntriestake an optional filter (defaultNone, so existing callers are unaffected), applied inshouldSkipEntry.The filter is applied in addition to
ignoredPathSegmentRegex(both must pass), so hidden entries stay hidden even when they match the glob.Why are the changes needed?
pathGlobFiltercannot express this. It is applied during file listing, so it must already match the archive file itself for that archive to be read at all; it cannot simultaneously select a subset of the entries inside the archive. WithoutarchivePathFilterthere is no way to read only part of an archive, so a user wanting a few entries has to read and discard the rest.Does this PR introduce any user-facing change?
Yes. A new read option
archivePathFilter(a glob string, no default) is available to the file sources that support archive reads. When set, only archive entries whose full inner path matches the glob are ingested. Reads that do not set it are unaffected, and the whole archive-read feature remains gated byspark.sql.files.archive.reader.enabled(defaultfalse).How was this patch tested?
New shared cases in
ArchiveReadSuiteBase, which run for every (format, container) pair -- CSV, JSON, XML, text, Avro, Parquet, ORC and binaryFile across tar, zip and 7z:sub/*),ignoredPathSegmentRegex(a hidden entry matching the glob is still skipped).New unit cases in
SupportsArchiveFormatSuitecover the entry-skip logic directly (glob matching the full path,*crossing/, no-match, interaction with hidden-entry filtering) and invalid-glob rejection.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code