rexminnis commented on PR #3046:
URL: https://github.com/apache/iceberg-rust/pull/3046#issuecomment-5874670479
As promised, a run against a real REST catalog: Apache Polaris 1.7.0 with S3
and vended credentials, on this branch plus hotcache#2.
**Setup.** The table was written by a different implementation. PyIceberg
0.12 created a format-v2 table partitioned by `identity(ts)` and appended three
files, one per distinct `ts`. It then evolved the spec to `day(ts)` and
appended one more file. That gives 400 rows, three spec-0 files in one
manifest, and one spec-1 file. The rewrites use `RewriteFilesAction` from this
branch. Trino 483 (Java) reads the result independently, and PyIceberg
cross-checks it.
1. **Rewrite 1:** two of the three spec-0 files are compacted into one
`day(ts)` file.
- The survivor is re-emitted `Existing` in a rewritten **spec-0**
manifest. It keeps its original snapshot id, `seq=1`, `file_seq=1` and
timestamptz partition value, and the manifest's `min_sequence_number` is 1.
- Trino's `$files` still shows the survivor as `spec_id 0` with
`ts=2026-08-24 17:00:00 UTC`.
- Totals are exact: 3 files and 400 rows.
- `changed-partition-count=3`, with `partitions.ts=…` for the two removed
files and `partitions.ts_day=2026-08-24` for the added one. That summary is the
hotcache#2 fix, now live.
2. **Rewrite 2:** the spec-0 survivor and the PyIceberg spec-1 file are
compacted into one file.
- Both emptied manifests are dropped, and no spec-0 file or manifest
remains.
- Totals chain exactly through both replaces: 400 rows and 2 files, and
`total-files-size` equals the two live files.
3. **Independent reads:** Trino gives the same answer at every step, and so
does time travel to each earlier snapshot. PyIceberg matches on the final
snapshot: `count=400`, `sum(id)=79800`, 400 distinct ids and `sum(v)=39900`.
**One gap the live run showed: removed files are not written as `DELETED`
entries.** `filter_manifests` drops the removed entries. Java's
`ManifestFilterManager` writes each one with `writer.delete(entry)`, an entry
with status `DELETED` owned by the removing snapshot. On this table, Trino's
`$manifests` reports `deleted_data_files_count = 0` for every manifest, while
the snapshot summary says `deleted-data-files=2`.
The practical consequence is in snapshot expiration. For a linear,
main-branch-only history, `RemoveSnapshots` picks `IncrementalFileCleanup`,
which finds data files to delete **only** through `DELETED` entries whose
snapshot has expired (`IncrementalFileCleanup#findFilesToDelete`). So a Java
`expireSnapshots` run on a table compacted by this action would never delete
the replaced data files; they'd stay in storage as orphans. Changelog readers
that walk manifest entries would miss the removals too.
`ManifestWriter::add_delete_entry` already exists, so writing the removed
entries as `DELETED` with the new snapshot id looks like a small change. It
does change one rule, though: a manifest whose entries were all removed by the
current snapshot still has to be written, holding only `DELETED` entries. The
next commit then drops it, as `ee3fbdd` already does for manifests left
all-deleted by an earlier snapshot. That matches Java, which keeps a manifest
if `snapshotId() == current`. `test_rewrite_files_removes_empty_manifest` and
the `retires_the_old_spec` fixture in hotcache#2 assert that the manifest is
absent. They would need to assert that it has no live entries instead. I'm
happy to adjust the fixture if you take this.
--
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]
