ArulJerald opened a new pull request, #3991: URL: https://github.com/apache/iceberg-python/pull/3991
The process-wide `ManifestFile` cache was keyed on `manifest_path` alone, and `ManifestFile` equality compares only that path, so the first read of a path won and any later read of the same path returned the earlier object. Two tables referencing one path therefore shared a single `ManifestFile`, and since its partition summaries, counts and sequence numbers drive scan pruning, a mismatched entry changed which files a scan considered. Closes #3978 # Rationale for this change The cache exists to avoid retaining duplicate objects when consecutive snapshots reference the same manifests, which is a memory optimisation worth keeping. The problem is only that a path match was treated as proof the content matched. `get_or_cache` now compares the full record before reusing a cached instance — falling back to `Record`'s structural equality, since `ManifestFile.__eq__` is intentionally path-only — and replaces the entry when the content differs. Genuine reuse still hits the cache; only a real collision takes the slow path. The added comparison costs up to roughly 16% of what constructing the record already costs on the same path (measured at 20 partition summaries: 3.3 µs against 20.8 µs), and is negligible for unpartitioned manifests. That seemed a fair price for not serving stale pruning metadata. ## Are these changes tested? Yes. `test_manifest_cache_detects_path_collision_with_different_content` in `tests/utils/test_manifest.py` caches one `ManifestFile`, then calls `get_or_cache` with a second sharing its `manifest_path` but carrying a different `added_snapshot_id` and `existing_files_count`, and asserts the second call returns the second object's content rather than the stale first. It then confirms a third call with content identical to what is cached still returns the cached instance, so the deduplication fast path is covered too. The test fails without this change and passes with it. Full file: 45 passed. ## Are there any user-facing changes? No. Scans that previously read mismatched manifest metadata for a colliding path now read the correct metadata; there is no API change. -- 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]
