hcrosse opened a new issue, #2341: URL: https://github.com/apache/iceberg-go/issues/2341
## Feature Request / Improvement I'd like a way for catalogs to report that a commit was rejected before anything was written, without treating it as a retryable conflict. In [`table/table.go:870–890`](https://github.com/apache/iceberg-go/blob/e904fb445780226acf43812afe78c1bd6ea6c80c/table/table.go#L870-L890), `doCommit` wraps every `CommitTable` error that does not match `ErrCommitFailed` in `commitStateUnknownError` and disables cleanup. `Transaction.Commit` then [marks the transaction committed](https://github.com/apache/iceberg-go/blob/e904fb445780226acf43812afe78c1bd6ea6c80c/table/transaction.go#L2642-L2655). That is appropriate for transport failures and ambiguous 5xx responses, since the catalog may have accepted the commit. It also applies to rejections that happen before any catalog write: - Metadata builder errors returned by [`UpdateAndStageTable`](https://github.com/apache/iceberg-go/blob/e904fb445780226acf43812afe78c1bd6ea6c80c/catalog/internal/utils.go#L310-L343) and [Hadoop's `CommitTable`](https://github.com/apache/iceberg-go/blob/e904fb445780226acf43812afe78c1bd6ea6c80c/catalog/hadoop/tables.go#L215-L247). This excludes the two stale-value checks, which a separate PR makes retryable. - Hadoop's rejection of a changed table location or the `write.metadata.location` property, [before it creates the metadata directory or writes metadata](https://github.com/apache/iceberg-go/blob/e904fb445780226acf43812afe78c1bd6ea6c80c/catalog/hadoop/tables.go#L220-L247). - REST rejection responses. With a valid error body, [400 maps to `ErrBadRequest` and 403 to `ErrForbidden`](https://github.com/apache/iceberg-go/blob/e904fb445780226acf43812afe78c1bd6ea6c80c/catalog/rest/rest.go#L724-L730). [`CommitTable` maps 404 to `catalog.ErrNoSuchTable`](https://github.com/apache/iceberg-go/blob/e904fb445780226acf43812afe78c1bd6ea6c80c/catalog/rest/rest.go#L1803-L1815), but all three still become unknown-state errors in `doCommit`. As a result, [cleanup skips any manifest lists rebuilt by this call](https://github.com/apache/iceberg-go/blob/e904fb445780226acf43812afe78c1bd6ea6c80c/table/table.go#L671-L701), and [the transaction refuses another commit](https://github.com/apache/iceberg-go/blob/e904fb445780226acf43812afe78c1bd6ea6c80c/table/transaction.go#L114-L124), even when the catalog wrote nothing. Java keeps these cases distinct. Both [`RESTTableOperations`](https://github.com/apache/iceberg/blob/6275107e38cc87a7d416ef231019ceb51556f196/core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java#L205-L234) and [`RESTSessionCatalog.commitTransaction`](https://github.com/apache/iceberg/blob/6275107e38cc87a7d416ef231019ceb51556f196/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java#L1538-L1555) use `ErrorHandlers.tableCommitHandler`. Its [commit handler](https://github.com/apache/iceberg/blob/6275107e38cc87a7d416ef231019ceb51556f196/core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java#L119-L139) throws `NoSuchTableException` for 404 and `CommitStateUnknownException` for 500, 502, 503 and 504. For 400, the [default handler](https://github.com/apache/iceberg/blob/6275107e38cc87a7d416ef231019ceb51556f196/core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java#L334-L345) throws `BadRequestException` or `IllegalArgumentException`, not `CommitS tateUnknownException`. I propose an exported sentinel such as `ErrCommitRejected` that catalogs can wrap when they know nothing was written. `doCommit` would return it without retrying, clean up only the files that this call created, and leave the transaction uncommitted. Unclassified errors would retain the conservative unknown-state behavior. Would you like this distinction? Would you prefer catalogs to opt in for each known rejection, or also have the REST catalog classify specific rejection status codes such as 400, 403 and 404 this way? Should a rejected transaction stay resumable, or become unusable since resubmitting the same changes would be rejected again? I'm happy to implement it. -- 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]
