hcrosse opened a new pull request, #2342: URL: https://github.com/apache/iceberg-go/pull/2342
Follows up #2100. **CI:** the `table` tests fail to compile on main until #2340 merges. This PR doesn't touch those call sites. I ran the full suite, race tests and lint with #2340's change applied, and they pass. ## What `MetadataBuilder.AddSnapshot` now wraps `table.ErrCommitFailed` when a snapshot's sequence number is not above the table's last sequence number, or on v3 when its first-row-id is behind next-row-id. The row-id error still matches `ErrInvalidRowLineage`. Other builder errors, including the timestamp checks, are unchanged. ## Why When a peer commits on another branch, the commit branch's requirement still holds, but the builder rejects the staged snapshot because the peer took its sequence number or row ids. The SQL, Glue, Hive and Hadoop catalogs apply updates on the client and returned that rejection as a plain error. `doCommit` treated it as an unknown commit state, so it didn't retry or clean up, and the transaction refused another `Commit`, although nothing was written and a refresh and rebuild would land it. Java throws `RetryableValidationException` for exactly these two checks ([sequence number](https://github.com/apache/iceberg/blob/6275107e38cc87a7d416ef231019ceb51556f196/core/src/main/java/org/apache/iceberg/TableMetadata.java#L1257-L1263), [first-row-id](https://github.com/apache/iceberg/blob/6275107e38cc87a7d416ef231019ceb51556f196/core/src/main/java/org/apache/iceberg/TableMetadata.java#L1285-L1289)), and `CatalogHandlers.commit` reports only that exception as a `CommitFailedException`, a 409 ([CatalogHandlers.java](https://github.com/apache/iceberg/blob/6275107e38cc87a7d416ef231019ceb51556f196/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java#L646-L656)). The timestamp checks stay terminal, as in Java, since the rebuild keeps the snapshot's timestamp. `RewriteDataFiles` with partial progress used to stop on these errors as an unknown state. It now records the group as failed and moves on to the next one. ## Testing - Builder test: both checks match `ErrCommitFailed`. The timestamp check and a missing first-row-id don't. - SQLite catalog test: after a peer appends to another branch, a main-branch append lands with retries enabled. With retries disabled, the first `Commit` returns `ErrCommitFailed` and the second lands. Covers the sequence number on v2 and first-row-id on v3. - `TestReplaceFilesWithRemovalsResumesAfterPeerOnAnotherBranch` no longer needs its test catalog to turn builder errors into conflicts. -- 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]
