dynamic merge conflict handling - #11643
Draft
macneale4 wants to merge 6 commits into
Draft
Conversation
The three-way differ classifies byte-identical edits as convergent and merges them before resolveCb runs (three_way_differ.go), and does the same for delete/delete. A hook placed at valueMerger.TryMerge therefore cannot express any rule that must conflict on agreeing edits. Add an optional RowMergePolicy consulted at every dsMatch branch, before classification. RowMergeDefer is the zero value and runs the existing code path unchanged, so a nil policy and an always-defer policy are both inert. resolveCb keeps its signature and meaning; the two preview call sites pass nil. Keyless tables always use the default reconciler: their rows carry a cardinality rather than an identity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MergeOpts gains a RowMergePolicy, bound to a table in MakeTableMerger the same way RecordViolationsForTables already is, and handed to the three-way differ. Installing a policy disables the fast prolly-tree merge. That path cannot offer a policy every three-way decision: tree.SendPatches elides convergent edits without calling its collision callback, and at level > 0 it skips whole identical subtrees without descending to rows at all. Making it consult a policy would mean giving up the elision that makes it fast, so a policy takes the differ path instead, which reaches every matched key because it diffs each side against the base independently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fast path skipped convergent edits in two places. At the leaf level SendPatches only called its collision callback when the two sides' values differed. Above the leaf level, two patches holding the same chunk address were skipped whole, so none of the rows inside were ever visited. Both now happen when SendPatches is asked to visit convergent edits: the leaf case consults the callback, and the identical-subtree case descends a level on both sides and reconsiders. The flag is off for every existing caller, so the default merge is unchanged. The fast path is no longer disabled when a policy is installed. It keeps eliding subtrees that only one side touched, which is the bulk of a typical merge; only regions both sides rewrote identically now cost a descent. A test asserts all 40 rows of an identically-rewritten region reach the policy; without the descent it sees none of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dolt builds its merged row through the value merger's result descriptor, so the row is in the merged schema by construction. A policy returns a bare tuple and has no such builder, which is unambiguous only when every side shares one schema. Defer whenever ThreeWayDiffInfo reports a schema change on either side or between the two, so such a merge keeps Dolt's own semantics rather than the collection's rule. The fast path already excludes schema changes, so this only affects the differ path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the four merge strictness levels as test policies over ordinary SQL rows, where a row is the document and a column is the field. They need no schema knowledge: val.Tuple.GetField gives per-column bytes, so the levels are byte comparisons. Each is a pure predicate that conflicts or defers, because deciding whether two edits collide is the level's question and composing a clean merge remains Dolt's. Nine cases run against all four levels on both merge paths, 72 cells. Four different answer sets from one interface is the point: any single level could be satisfied by a hook with less information, all four together cannot. Two cases are the incomparability witnesses -- disjoint columns merge under fieldTouched and conflict under documentDivergent, an identical edit does the reverse. Also: a convergent delete now reaches the fast path's callback, and TryMerge has no answer for a row neither side kept. Left already reflects the deletion, so send no patch and record no conflict, as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three tuples are not enough when a table keeps its whole document in one value column, which is dumbodb's shape: a single adaptive blob. Two things were missing. Reading. An adaptive field holds either inline bytes or an out-of-band pointer, so reading the field directly works for small values and misreads large ones. Resolving it needs the value descriptor and the node store. Composing. With one document per column, a rule that wants to merge two edits to different fields inside that document cannot defer: at tuple granularity one column changed differently on both sides, so Dolt's cell rule conflicts, which is the opposite answer. The merged document is a value neither side holds, so the policy has to build it. The merge-level policy now takes a RowMergeInput struct carrying the table, the three tuples, the merged value descriptor and the node store. A struct rather than a parameter list, because this list has already proven under-specified once. The storage-layer callback stays tuple-only. Tests add a single-adaptive-value table where a field-level rule decodes, composes and returns a built tuple. Its precondition asserts that a deferring policy conflicts on the same input, which is what makes composition necessary rather than convenient, and one case asserts the value really did spill out of band rather than assuming 16KB is over the threshold. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
@macneale4 DOLT
|
Contributor
|
@macneale4 DOLT
|
Contributor
|
@macneale4 DOLT
|
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
2 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.
No description provided.