Skip to content

Commit 59b7c1c

Browse files
porkedchopkevinjqliuCopilot
authored
Validate snapshot sequence numbers on v3 tables (#4125)
* fix(table): validate snapshot sequence numbers on v3 tables The check that rejects a snapshot whose sequence number is not greater than the table's last sequence number only ran for format version 2. Java applies it to every format version after 1. * test: derive expected sequence number from the fixture The stale sequence-number test built the snapshot from the fixture's last_sequence_number but hardcoded 34 in the expected error message. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Kevin Liu <kevin.jq.liu@gmail.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 41006bf commit 59b7c1c

2 files changed

Lines changed: 25 additions & 1 deletion

File tree

‎pyiceberg/table/update/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ def _(update: AddSnapshotUpdate, base_metadata: TableMetadata, context: _TableMe
453453
elif base_metadata.snapshot_by_id(update.snapshot.snapshot_id) is not None:
454454
raise ValueError(f"Snapshot with id {update.snapshot.snapshot_id} already exists")
455455
elif (
456-
base_metadata.format_version == 2
456+
base_metadata.format_version >= 2
457457
and update.snapshot.sequence_number is not None
458458
and update.snapshot.sequence_number <= base_metadata.last_sequence_number
459459
and update.snapshot.parent_snapshot_id is not None

‎tests/table/test_init.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2084,6 +2084,30 @@ def test_add_snapshot_update_fails_with_smaller_first_row_id(table_v3: Table) ->
20842084
update_table_metadata(table_v3.metadata, (AddSnapshotUpdate(snapshot=new_snapshot),))
20852085

20862086

2087+
@pytest.mark.parametrize("table", [lf("table_v2"), lf("table_v3")])
2088+
def test_add_snapshot_update_fails_with_stale_sequence_number(table: Table) -> None:
2089+
last_sequence_number = table.metadata.last_sequence_number
2090+
new_snapshot = Snapshot(
2091+
snapshot_id=25,
2092+
parent_snapshot_id=3055729675574597004,
2093+
sequence_number=last_sequence_number,
2094+
timestamp_ms=1602638593590,
2095+
manifest_list="s3:/a/b/c.avro",
2096+
summary=Summary(Operation.APPEND),
2097+
schema_id=1,
2098+
first_row_id=2,
2099+
)
2100+
2101+
with pytest.raises(
2102+
ValueError,
2103+
match=(
2104+
f"Cannot add snapshot with sequence number {last_sequence_number} "
2105+
f"older than last sequence number {last_sequence_number}"
2106+
),
2107+
):
2108+
update_table_metadata(table.metadata, (AddSnapshotUpdate(snapshot=new_snapshot),))
2109+
2110+
20872111
def test_add_snapshot_update_updates_next_row_id(table_v3: Table) -> None:
20882112
new_snapshot = Snapshot(
20892113
snapshot_id=25,

0 commit comments

Comments
 (0)