cayetanobv opened a new pull request, #4102:
URL: https://github.com/apache/iceberg-python/pull/4102
Closes #3621 (part of #1551). Stacked on #3624, which adds
`ManifestListWriterV3`. Only the last commit belongs to this PR; I will rebase
once #3624 merges.
# Rationale for this change
Every commit to a V3 table fails on a REST catalog.
`_SnapshotProducer._commit` sets `first-row-id` on the new snapshot but never
sets `added-rows`, and the spec requires both together. The REST fixture
rejects the snapshot:
```
java.lang.IllegalArgumentException: Invalid added-rows (required when
first-row-id is set): null
```
Without a server check, the table metadata goes wrong instead.
`add_snapshot` computes `next-row-id` from `added-rows`, so `next-row-id`
becomes `None` after the first commit and the next commit fails.
This PR sets `added-rows` to `writer.next_row_id - first_row_id`, which is
the number of row IDs `ManifestListWriterV3` assigned. Java's
`SnapshotProducer` computes it the same way. The count includes existing rows
in manifests that had no `first_row_id`, so `next-row-id` always moves past
every assigned ID.
It also removes `_calculate_added_rows`. Nothing called it, and it counted
only `added_rows_count` from the snapshot's own manifests, which would
undercount the rows that the writer assigns.
Out of scope: a V3 table created by `new_table_metadata` has `next-row-id =
None`, so a commit to it still fails. A REST server sets it to 0 on create,
which hides this. That belongs with writing V3 metadata (#3551).
## Are these changes tested?
Yes. Three new tests in `tests/table/test_snapshots.py` commit to an empty
V3 table inside a transaction, because writing a V3 `metadata.json` is not
supported yet:
- two appends: `(first_row_id, added_rows)` is `(0, 3)`, then `(3, 3)`, and
`next-row-id` is 6
- `add_files`: `(0, 3)`, `next-row-id` is 3
- a delete that rewrites a data file: the 2 surviving rows get new IDs, `(3,
2)`, and `next-row-id` is 5
All three fail without the change. The unit suite passes locally.
This PR also un-skips the integration test
`test_v3_write_and_read_row_lineage`. It now appends twice and reads `_row_id`
and `_last_updated_sequence_number` with Spark (Iceberg 1.11.0), which must see
row IDs 0 to 5 with sequence numbers 1 and 2. It passes with the change and
fails without it on the same `Invalid added-rows` error.
I also ran this script against `apache/iceberg-rest-fixture`
(`sha256:db8de90b…`) with the RustFS warehouse from
`dev/docker-compose-integration.yml`:
```python
t = cat.create_table(ident, schema=data.schema,
properties={"format-version": "3"})
t.add_files([f"file://{src}"]) # 3 rows
cat.load_table(ident).append(data) # 3 rows
cat.load_table(ident).delete("id == 1") # rewrites both files
```
Before, on #3624 (`c152089d`):
```
lineage.t_2890d52f format-version=3 next-row-id=0
CommitStateUnknownException: RESTError 500: ... Invalid added-rows (required
when first-row-id is set): null
```
After:
```
lineage.t_0e297eff format-version=3 next-row-id=0
add_files: first-row-id=0 added-rows=3 next-row-id=3 rows=3
append : first-row-id=3 added-rows=3 next-row-id=6 rows=6
delete : first-row-id=6 added-rows=4 next-row-id=10 rows=4
```
## Are there any user-facing changes?
No API changes. Commits to V3 tables through a REST catalog now succeed.
Written with Claude Code (Claude Opus 5.5) and reviewed by me. One path has
no test: `added-rows` for tables upgraded from V2, which follows Java but needs
#3622 to exercise.
--
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]
