kevinjqliu opened a new issue, #3814:
URL: https://github.com/apache/iceberg-python/issues/3814
## Description
Follow-up to #3811.
An overwrite can explicitly delete a `DataFile` that is already absent from
its base snapshot. Because `_validate_data_files_exist` only examines the
commit window, the missing file is not detected when its deletion occurred
before that window.
This requires a stale `DataFile` supplied through the low-level snapshot
API; the high-level overwrite and upsert paths are not affected.
The commit succeeds, adds the replacement file, and can reintroduce data
derived from the deleted file. It also produces incorrect snapshot summary
totals.
## Reproduction
The following can be added to `tests/table/test_commit_retry.py`:
```python
def test_overwrite_rejects_file_missing_from_base(catalog: Catalog) -> None:
import pyarrow as pa
from pyiceberg.io.pyarrow import _dataframe_to_data_files
catalog.create_namespace("default")
identifier = "default.stale_file_overwrite"
table = catalog.create_table(
identifier,
Schema(NestedField(1, "x", LongType(), required=False)),
)
table.append(pa.table({"x": [1]}))
stale_table = catalog.load_table(identifier)
stale_file = stale_table.scan().plan_files()[0].file
stale_rows = stale_table.scan().to_arrow()
# Delete F before the replacement transaction begins.
with catalog.load_table(identifier).transaction() as tx:
with tx.update_snapshot().overwrite() as overwrite:
overwrite.delete_data_file(stale_file)
current = catalog.load_table(identifier)
replacement = next(
_dataframe_to_data_files(
table_metadata=current.metadata,
df=stale_rows,
io=current.io,
write_uuid=uuid.uuid4(),
)
)
# This currently succeeds even though stale_file is absent from the base.
with current.transaction() as tx:
with tx.update_snapshot().overwrite() as overwrite:
overwrite.delete_data_file(stale_file)
overwrite.append_data_file(replacement)
committed = catalog.load_table(identifier)
assert committed.scan().to_arrow()["x"].to_pylist() == [1]
assert len(committed.scan().plan_files()) == 1
assert
committed.current_snapshot().summary.additional_properties["total-data-files"]
== "0"
```
Expected behavior: raise `ValidationException` because the explicitly
deleted file is missing.
Actual behavior: the replacement commits, the deleted data reappears, and
the summary reports zero data files despite one live file.
## Java reference
Java handles this separately from commit-window validation:
- [`BaseOverwriteFiles` enables
`failMissingDeletePaths`](https://github.com/apache/iceberg/blob/bb9b9a06b3fb320c3496f4bac9fbf455f7a773b2/core/src/main/java/org/apache/iceberg/BaseOverwriteFiles.java#L115-L126).
-
[`ManifestFilterManager.validateRequiredDeletes`](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/ManifestFilterManager.java)
verifies that explicitly deleted files exist in the current manifests.
-
[`validateDataFilesExist`](https://github.com/apache/iceberg/blob/bb9b9a06b3fb320c3496f4bac9fbf455f7a773b2/core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java#L772-L822)
separately validates removals within the commit window.
## Suggested fix
While planning an overwrite, compare the explicitly requested files with
those found in the current parent snapshot and raise `ValidationException` for
any missing files. The existing manifest scan used to build deleted entries can
provide this check without scanning snapshot history.
--
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]