fix(table): validate snapshot timestamp drift on add snapshot

[mrutunjay-kinagi]

Rationale for this change

Align snapshot timestamp validation with other Iceberg implementations by rejecting snapshots that drift backwards by more than one minute.

The new guard in AddSnapshotUpdate checks:

• snapshot timestamp vs latest snapshot_log entry
• snapshot timestamp vs table last_updated_ms

Both checks allow up to 60 seconds of clock skew tolerance.

Are these changes tested?

Yes.

• Added test_update_metadata_add_snapshot_rejects_old_timestamp_vs_snapshot_log.
• Added test_update_metadata_add_snapshot_rejects_old_timestamp_vs_last_updated.
• Verified with targeted pytest run.

Are there any user-facing changes?

No API changes.

• Invalid commit metadata now fails fast with explicit ValueError when timestamp drift exceeds tolerance.

[Fokko]

@mrutunjay-kinagi Can you explain more about the use-case? I'm hesitant with these kind of features since it isn't implemented in other OSS implementations AFAIK.

[Fokko]

Maybe CommitFailedException?

[mrutunjay-kinagi]

Good point. I kept ValueError for consistency with the existing validation branches in this same method (sequence/row-id checks all currently raise ValueError). If you prefer, I can switch this validation path to CommitFailedException in a follow-up commit.

[mrutunjay-kinagi]

@Fokko Thanks for the review. The use case here is handling malformed/clock-skewed commit metadata safely and aligning behavior with Java/Rust, which already apply a 1-minute backward-drift tolerance for snapshot timestamps.\n\nWithout this guard, a client can add a snapshot timestamp that is significantly older than current table metadata ( / latest ), which can make timeline semantics inconsistent for timestamp-based operations.\n\nThis change keeps the same 60s tolerance and only rejects larger backward drift; small skew remains allowed.

[mrutunjay-kinagi]

Clarification on my previous comment (formatting issue):

The guard is intended to prevent adding snapshots that are significantly older than the table timeline, specifically older than last_updated_ms or the latest snapshot_log entry by more than 60 seconds.

This follows the same tolerance approach used in Java/Rust and keeps small clock skew allowed while rejecting larger backward drift.

[kevinjqliu]

This follows the same tolerance approach used in Java/Rust and keeps small clock skew allowed while rejecting larger backward drift.

could you point to the java/rust references?

[mrutunjay-kinagi]

The existing validation branches inside are raising , so for symmetry I left this guard as as well. If you’d prefer switching to later that’s easy to follow up on but I’d keep the current behavior for consistency.

[mrutunjay-kinagi]

Reposting cleanly: the guard maintains the same exception type as the surrounding validation branches, so I kept it as ValueError for consistency. If you’d prefer CommitFailedException later, happy to follow up.

[mrutunjay-kinagi]

@kevinjqliu Here are the references we aligned with:

• Java enforces the same tolerance inside TableMetadata’s constructor (lastUpdatedMillis - last.timestampMillis >= -ONE_MINUTE): https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/TableMetadata.java#L360-L377
• Rust’s TableMetadataBuilder applies the same check before adding a snapshot (snapshot.timestamp_ms must not be more than 1 minute behind the latest log entry or last_updated_ms): https://github.com/apache/iceberg-rust/blob/main/crates/iceberg/src/spec/table_metadata_builder.rs#L371-L399