hcrosse commented on code in PR #2350:
URL: https://github.com/apache/iceberg-go/pull/2350#discussion_r4415412882


##########
table/table.go:
##########
@@ -62,6 +62,15 @@ import (
 // stops.
 var ErrCommitFailed = errors.New("commit failed, refresh and try again")
 
+// ErrCommitRejected means the catalog rejected a commit without writing
+// anything. It is the Go equivalent of Java's CleanableFailure.
+//
+// Transaction.Commit does not retry it, but a later Commit can try again.
+// Staged files are retained. Files created during retries are deleted unless
+// the transaction can reuse them. Errors that also match ErrCommitFailed
+// are treated as conflicts.
+var ErrCommitRejected = errors.New("commit rejected")

Review Comment:
   Thanks, I added the note to the `ErrCommitRejected` doc comment in 9cb9fd6.



##########
catalog/rest/rest.go:
##########
@@ -114,6 +114,12 @@ var (
        ErrCommitStateUnknown = fmt.Errorf("%w: commit failed due to unknown 
reason", ErrRESTError)
        ErrOAuthError         = fmt.Errorf("%w: oauth error", ErrRESTError)
 
+       // These statuses mean the catalog did not change the table.
+       errCommitBadRequest   = fmt.Errorf("%w: %w", ErrBadRequest, 
table.ErrCommitRejected)
+       errCommitUnauthorized = fmt.Errorf("%w: %w", ErrUnauthorized, 
table.ErrCommitRejected)
+       errCommitForbidden    = fmt.Errorf("%w: %w", ErrForbidden, 
table.ErrCommitRejected)
+       errCommitNoSuchTable  = fmt.Errorf("%w: %w", catalog.ErrNoSuchTable, 
table.ErrCommitRejected)

Review Comment:
   Good call. I added a compatibility note to `website/src/releases.md` in 
9cb9fd6.



##########
catalog/internal/utils.go:
##########
@@ -281,7 +281,22 @@ func ValidateRequirements(ident table.Identifier, current 
table.Metadata, reqs [
        return nil
 }
 
-func UpdateAndStageTable(ctx context.Context, catprops iceberg.Properties, 
current *table.Table, ident table.Identifier, reqs []table.Requirement, updates 
[]table.Update, cat table.CatalogIO) (*table.StagedTable, error) {
+// RejectUnlessConflict wraps non-conflict errors with table.ErrCommitRejected.
+// Use it only before the catalog has written anything.
+func RejectUnlessConflict(err error) error {

Review Comment:
   Thanks for flagging this. The same `releases.md` note now covers the error 
text change, also in 9cb9fd6.



-- 
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]

Reply via email to