Skip to content

feat: introduce IncompatibleTransaction error#6003

Merged
wjones127 merged 2 commits intolance-format:mainfrom
wjones127:feat/differentiate-incompat-transaction
Feb 24, 2026
Merged

feat: introduce IncompatibleTransaction error#6003
wjones127 merged 2 commits intolance-format:mainfrom
wjones127:feat/differentiate-incompat-transaction

Conversation

@wjones127
Copy link
Copy Markdown
Contributor

From #5983 (comment), we currently use CommitConflict for two situations:

  1. Incompatible transactions: there is a conflict that is not retry-able. For example, you are trying to create an index, but a concurrent transaction overwrote the table and changed the schema.
  2. Commit step ran out of retries: we hit the max number of rebase attempts, and even though we could retry again, we aren't. This is indeed just throttling.

This makes them separate errors.

@github-actions github-actions Bot added the enhancement New feature or request label Feb 24, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Code Review: PR #6003 - feat: introduce IncompatibleTransaction error

Summary

This PR introduces a new IncompatibleTransaction error type to distinguish between non-retryable transaction conflicts and conflicts that exhausted retries. The semantic separation is correct and improves error clarity.

P1 Issue: Java bindings inconsistency

The Java error mapping at java/lance-jni/src/error.rs:207 explicitly handles CommitConflictIllegalArgumentException, but the new IncompatibleTransaction error will fall through to the default case (RuntimeException):

LanceError::DatasetNotFound { .. }
| LanceError::DatasetAlreadyExists { .. }
| LanceError::CommitConflict { .. }  // ← explicitly handled
| LanceError::InvalidInput { .. } => Self::input_error(err.to_string()),
// IncompatibleTransaction falls through to RuntimeException

Since IncompatibleTransaction represents a semantic transaction conflict (arguably more severe than CommitConflict), it should likely be mapped consistently. Consider adding LanceError::IncompatibleTransaction { .. } to the same match arm, or make a deliberate choice about how Java clients should handle this new error type.


The rest of the changes look correct - the error variant is well-defined, tests have been updated appropriately, and the error message provides good context about the incompatibility.

@wjones127 wjones127 requested a review from jackye1995 February 24, 2026 23:01
@wjones127 wjones127 marked this pull request as ready for review February 24, 2026 23:01
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 24, 2026

Codecov Report

❌ Patch coverage is 38.09524% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance/src/io/commit/conflict_resolver.rs 38.46% 0 Missing and 8 partials ⚠️
rust/lance/src/index/mem_wal.rs 50.00% 0 Missing and 3 partials ⚠️
rust/lance/src/io/commit.rs 0.00% 0 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@wjones127 wjones127 merged commit 4af7726 into lance-format:main Feb 24, 2026
29 checks passed
wjones127 added a commit to wjones127/lance that referenced this pull request Feb 25, 2026
From
lance-format#5983 (comment),
we currently use `CommitConflict` for two situations:

1. Incompatible transactions: there is a conflict that is not
retry-able. For example, you are trying to create an index, but a
concurrent transaction overwrote the table and changed the schema.
1. Commit step ran out of retries: we hit the max number of rebase
attempts, and even though we could retry again, we aren't. This is
indeed just throttling.

This makes them separate errors.
wjones127 added a commit to wjones127/lance that referenced this pull request Feb 25, 2026
From
lance-format#5983 (comment),
we currently use `CommitConflict` for two situations:

1. Incompatible transactions: there is a conflict that is not
retry-able. For example, you are trying to create an index, but a
concurrent transaction overwrote the table and changed the schema.
1. Commit step ran out of retries: we hit the max number of rebase
attempts, and even though we could retry again, we aren't. This is
indeed just throttling.

This makes them separate errors.
wjones127 added a commit that referenced this pull request Feb 26, 2026
From
#5983 (comment),
we currently use `CommitConflict` for two situations:

1. Incompatible transactions: there is a conflict that is not
retry-able. For example, you are trying to create an index, but a
concurrent transaction overwrote the table and changed the schema.
1. Commit step ran out of retries: we hit the max number of rebase
attempts, and even though we could retry again, we aren't. This is
indeed just throttling.

This makes them separate errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants