docs: document the rules for transaction conflicts#6158
docs: document the rules for transaction conflicts#6158westonpace merged 2 commits intolance-format:mainfrom
Conversation
github-actions
Bot
added
the
documentation
PR ReviewThis is a well-structured documentation improvement that replaces an incomplete conflict table in code comments with a thorough markdown specification. The directional framing (from the perspective of the operation being committed) is a good call and addresses a common source of confusion. Two issues to flag: 1. Merge Compatibility lists Project as retryable, but code treats it as incompatible (P1)In the "Merge Compatibility" section, 2. Unresolved TODO should not be merged (P0)Under "CreateIndex Compatibility", there is: This should be resolved before merging — either verify and document the behavior, or remove it with a tracking issue. 🤖 Generated with Claude Code |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| If two CreateIndex operations are committed concurrently then it is allowed. If the indexes have different names this is no | ||
| problem. If the indexes have the same name then the second operation will win and replace the first. |
There was a problem hiding this comment.
This seems accurate to the current implementation. Though I think we should probably change it so you can commit index segments for disjoint fragment sets and it be additive.
There was a problem hiding this comment.
Now that we are allowed multiple UUIDs per name that does make sense.
|
|
||
| - Rewrite (only if overlapping fragments, no stable row ids, and no fragment reuse index) | ||
| - DataReplacement (only if overlapping fragments and the column being replaced is being indexed) | ||
| - Merge ??? TODO - test this |
There was a problem hiding this comment.
Do we have an answer for this?
There was a problem hiding this comment.
Yes, it is not a conflict. I was originally thinking merge == update and I was confused how it couldn't be if the merge could replace a column. However, the logic in merge seems to assume we are only ever adding columns and so it considers it ok.
2 participants
There was a conflict table in transaction.rs but this was incomplete (some rows/columns missing) and seemed to be imprecise or incorrect in a few spots. I've attempted to more thoroughly document this in transaction.md instead.