laichunpongben opened a new pull request, #3287:
URL: https://github.com/apache/iceberg-python/pull/3287
Fixes #3286.
## What
`Transaction.add_files(check_duplicate_files=True)` previously materialized
every DataFile in the current snapshot via `inspect.data_files()` and
post-filtered on `file_path`. For incremental backfills (repeated `add_files`
on a growing table), per-call cost is O(snapshot file count); cumulative cost
is O(N²).
This PR replaces the materialization with a streaming scan over manifest
entries that short-circuits on `file_path` containment in a candidate `set`.
Delete manifests are skipped at the top level. No partition records /
readable_metrics / stats decoding for the dup-check path.
The same approach Spark's `add_files` action uses — predicate-based against
new paths only.
## Why
- The full `inspect.data_files()` call decodes per-column lower/upper bounds
via `from_bytes` for every column of every DataFile
(`pyiceberg/table/inspect.py:_get_files_from_manifest`), then builds a pyarrow
Table per manifest, then concats and filters in memory. None of that is needed
to answer "is `file_path` in the snapshot?".
- For a backfill that calls `add_files` once per day across a month-long
window, dup-check time grows linearly per call and dominates total wall-clock
by the second half of the run.
- Setting `check_duplicate_files=False` (the workaround in #2132 / docs PR
#2249) loses idempotency for partial-failure resume.
## What this is and isn't
- **Is**: a constant-factor reduction (skip stats decoding, skip pyarrow
round-trip, skip partition decode, top-level filter on manifest content).
Per-call still O(snapshot file count) for the manifest entry reads.
- **Isn't**: an asymptotic fix. Truly eliminating the linear scan would need
`file_path` lower/upper bounds at the `ManifestFile` level so most manifests
can be pruned without opening — that's a spec extension and a follow-up.
The constant factor matters in practice because the avoided work
(per-column-stats decode + pyarrow materialization) was the bulk of the time,
not the Avro entry read.
## Compatibility
- Public API unchanged (`add_files` signature and exception message
identical).
- Existing integration tests at
`tests/integration/test_add_files.py:test_add_files_that_referenced_by_current_snapshot{,_with_check_duplicate_files_true,_with_check_duplicate_files_false}`
exercise the dup-check contract and continue to pass.
## Refs
- Issue: #3286
- Related: #2132 (closed as docs), #2133 (parallelization)
--
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]
