feat: add automatic conflict resolution support for delete operations#4407
Merged
wjones127 merged 4 commits intolance-format:mainfrom Aug 19, 2025
Merged
Conversation
Add affected_rows tracking to delete operations to enable automatic conflict resolution similar to update operations. This allows concurrent delete operations to succeed with proper retry-based conflict resolution. Changes: - Add affected_rows field to DeleteData struct - Capture RowIdTreeMap during delete execution from removed row addresses - Pass affected_rows to CommitBuilder.with_affected_rows() in commit step - Add test_delete_concurrency test similar to test_update_concurrency - Handle edge cases for delete-all and delete-nothing scenarios 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
wjones127
force-pushed
the
feat/delete-conflict-resolution
branch
from
27e697c to
232fba2
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4407 +/- ##
==========================================
+ Coverage 82.21% 82.24% +0.02%
==========================================
Files 307 307
Lines 124969 125188 +219
Branches 124969 125188 +219
==========================================
+ Hits 102739 102956 +217
- Misses 18387 18394 +7
+ Partials 3843 3838 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jackye1995
approved these changes
Aug 19, 2025
Contributor
jackye1995
left a comment
jackye1995
left a comment
There was a problem hiding this comment.
looks good to me, just a comment regarding testing
| (Vec::new(), deleted_fragment_ids) | ||
| // When deleting everything, we don't have specific row addresses, | ||
| // but we can create an empty RowIdTreeMap since all fragments are deleted | ||
| (Vec::new(), deleted_fragment_ids, RowIdTreeMap::new()) |
Contributor
There was a problem hiding this comment.
I think the logic should be right here based on the conflict_resolver logic, but can we add a test to confirm?
Contributor
Author
There was a problem hiding this comment.
Good catch. Added a test and modified this not to return an affected_rows here.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds retry-based conflict resolution for delete operations, similar to update operations.
Previously, concurrent deletes would fail with commit conflicts. Now they automatically retry.
Fixes #3775