laskoviymishka commented on code in PR #2100:
URL: https://github.com/apache/iceberg-go/pull/2100#discussion_r4389734930
##########
table/table.go:
##########
@@ -684,39 +732,62 @@ func (t Table) doCommit(ctx context.Context, updates
[]Update, reqs []Requiremen
}
if t.metadata != nil && fresh.metadata != nil {
if err := validateTableUUID(t.identifier,
t.metadata.TableUUID(), fresh.metadata.TableUUID()); err != nil {
- return nil, err
+ return nil, fmt.Errorf("%w: table was
replaced: %w", ErrTransactionUnusable, err)
}
}
current = fresh.metadata
- reqs = rewriteRefSnapshotRequirements(reqs, co.branch,
current, co.pinnedRefs)
+ reqs = rewriteRefSnapshotRequirements(reqs, co.branch,
current, pinned)
- // A pinned assertion the fresh catalog state violates
can
- // never succeed — fail now instead of burning the
remaining
- // retries on it.
- if err := validatePinnedRefRequirements(reqs,
co.pinnedRefs, current); err != nil {
- return nil, fmt.Errorf("%w: explicit ref
requirement failed: %w", ErrCommitFailed, err)
+ if err := validateNonRebasedRequirements(reqs,
co.branch, pinned, current); err != nil {
+ if co.noReplay {
+ return nil, fmt.Errorf("%w: requirement
no longer holds after refresh (%s): %w",
+ ErrTransactionUnusable,
noReplayHint, err)
+ }
+
+ return nil, fmt.Errorf("%w: requirement no
longer holds after refresh: %w", ErrTransactionUnusable, err)
Review Comment:
That works for me. The two-question framing is clearer than the retry-only
line I was pushing. Could the `ErrCommitFailed` doc carry it close to how you
wrote it here, and spell out the split from your walk-through: stale
requirements, validator conflicts and a deleted branch match it, while a tag
target, a replaced table and a diverged base don't?
Yes please to the bounded external-loop test with both outcomes. That's the
one thing I'd still want before merge.
--
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]
