Skip to content

Document merge conflicts on two branches editing different sub-fields of one nested document #85

Description

Two branches edit two different sub-fields of one nested document. Neither
touches the other's key. The merge conflicts.

The same edit shape one level up — two different top-level fields — merges
cleanly, which is what makes this look like a granularity gap rather than a
policy choice.

Repro

repro-nested-document-merge.mjs, run against a fresh data dir on defaults (no
--session-isolation):

// repro: two branches edit two DIFFERENT sub-fields of one nested document.
// Neither touches the other's key. The merge conflicts anyway.
//
//   npm i mongodb
//   DUMBO_URL=mongodb://127.0.0.1:27017 DUMBO_DB=repro node repro-nested-document-merge.mjs
//
// against a fresh dumbodb data dir (defaults; no --session-isolation).
import { MongoClient } from "mongodb";

const URL = process.env.DUMBO_URL;
const DB = process.env.DUMBO_DB;
if (!URL || !DB) {
  console.error("usage: DUMBO_URL=mongodb://host:port DUMBO_DB=<database> node repro-nested-document-merge.mjs");
  process.exit(1);
}

const client = await new MongoClient(URL, { directConnection: true }).connect();
const main = client.db(DB);
const feat = client.db(DB + "@feature");
const commit = (db, message) => db.command({ dumboCommit: 1, message, author: "repro <repro@localhost>" });

// One document with a nested sub-document holding two independent keys.
await main.collection("docs").insertOne({ _id: 1, meta: { owner: "alice", region: "eu" } });
await commit(main, "seed");

await main.command({ dumboBranch: 1, action: "add", branch: "feature" });

// feature touches meta.region only.  main touches meta.owner only.
await feat.collection("docs").updateOne({ _id: 1 }, { $set: { "meta.region": "us" } });
await commit(feat, "feature: region -> us");

await main.collection("docs").updateOne({ _id: 1 }, { $set: { "meta.owner": "bob" } });
await commit(main, "main: owner -> bob");

const show = async (label, fn) => {
  try { console.log(label, JSON.stringify(await fn())); }
  catch (e) { console.log(label, JSON.stringify(e.errorResponse ?? { errmsg: e.message })); }
};

console.log("--- NESTED: feature set meta.region, main set meta.owner ---");
await show("merge:", () => main.command({ dumboMerge: 1, mergeIn: "feature" }));
await show("conflicts:", () => main.command({ dumboConflicts: 1 }));
console.log("doc after merge:", JSON.stringify(await main.collection("docs").findOne({ _id: 1 })));
await main.command({ dumboMerge: 1, abort: 1 }).catch(() => {});

// Control: the SAME edit shape one level up merges cleanly.
await main.collection("flat").insertOne({ _id: 1, owner: "alice", region: "eu" });
await commit(main, "seed flat");
await main.command({ dumboBranch: 1, action: "add", branch: "feature2" });
const feat2 = client.db(DB + "@feature2");
await feat2.collection("flat").updateOne({ _id: 1 }, { $set: { region: "us" } });
await commit(feat2, "feature2: region -> us");
await main.collection("flat").updateOne({ _id: 1 }, { $set: { owner: "bob" } });
await commit(main, "main: owner -> bob (flat)");
console.log("--- CONTROL (same shape, top level): feature2 set region, main set owner ---");
await show("merge:", () => main.command({ dumboMerge: 1, mergeIn: "feature2" }));
console.log("doc after merge:", JSON.stringify(await main.collection("flat").findOne({ _id: 1 })));

await client.close();

Observed — v0.6.3 (7b226da)

--- NESTED: feature set meta.region, main set meta.owner ---
merge: {"conflicts":[{"collection":"docs","count":1}],"ok":0,"code":96,
        "errmsg":"dumboMerge: unresolved conflicts in 1 collection(s)"}

conflicts: {"conflicts":[{"conflictId":"828hAE2KautL0+sFPTrj0g","type":"document",
  "collection":"docs",
  "reason":{"code":"bothModified",
            "message":"branch 'main' (ours) and branch 'feature' (theirs) both modified document 1"},
  "base":  {"_id":1,"doc":{"_id":1,"meta":{"owner":"alice","region":"eu"}}},
  "ours":  {"_id":1,"doc":{"_id":1,"meta":{"owner":"bob","region":"eu"}},"diffType":"modified"},
  "theirs":{"_id":1,"doc":{"_id":1,"meta":{"owner":"alice","region":"us"}},"diffType":"modified"}}],
  "ok":1}

The conflict envelope contains the evidence: ours changed meta.owner and
left meta.region at its base value; theirs changed meta.region and left
meta.owner at its base value. No key was written by both sides.

Control, same script, same run — the identical edit shape at top level:

--- CONTROL (same shape, top level): feature2 set region, main set owner ---
merge: {"commitId":"g66vokouid8icfhcv42qpc6djef9vuhd", ... "ok":1}
doc after merge: {"_id":1,"owner":"bob","region":"us"}

Cause

mergeBSONDoc (internal/backends/dolt/bson_merge.go) enumerates top-level
keys only
and compares values with reflect.DeepEqual. A nested
*types.Document is therefore an atomic value: both sides touched it, the two
values differ, and the same-key arm falls through to default: return nil, true.

This is a regression rather than an original limitation. The resolve callback
supplied to tree.NewThreeWayDiffer used to run a recursive structural merge —
mergeJSON, the maintained copy of dolt's MergeJSON in
internal/backends/dolt/json_merge.go. 473ebd6 ("backends/dolt: integrate
bson-a storage format") replaced the callback's body:

-		mergedDoc, conflict, err := mergeJSON(ctx, ns, baseDoc, leftDoc, rightDoc)
+		mergedDoc, conflict := mergeBSONDoc(baseDoc, leftDoc, rightDoc)

and 20163f1 deleted the then-dead json_merge.go. The hook itself was never
removed and is unchanged.

A separate, smaller finding in the same place

mergeBSONDoc's doc comment, as written in 473ebd6, says:

Deeply nested container conflicts fall back to the dolt prolly three-way
differ at a higher level when the field-level test reports conflict.

There is no such fallback. At the divergent-clash arm
($dolt/go/store/prolly/tree/three_way_differ.go:203):

resolved, ok, err := d.resolveCb(...)
if !ok {
    res = d.newDivergentClashConflict(...)
}

Declining the callback records a conflict; nothing retries at a higher level,
and nothing can look inside the value because the whole document is one cell.
Worth correcting regardless of what happens to the rest of this — the comment is
what makes the flat walk read as safe.

Suggested rule

Where base, ours and theirs all hold a document at the same key, recurse: the
same union-of-keys walk and the same conflict rule one level down. Fields are
already lex-sorted at every level on write (bson_codec.go), so the canonical
form MergeJSON had to construct for itself already holds, which makes this
about fifteen lines rather than a port.

Two limits worth stating explicitly, because they are choices:

  • Arrays stay whole values. A positional three-way merge has to decide what
    an index means when one side inserted and the other edited, and every answer
    to that invents data.
  • A document on one side and a scalar on the other still conflicts. There is
    no merged shape for that.

This is also, we think, the "companion answer for arrays" that
docs/design/merge-strictness.md §9 asks for under "What counts as a field" —
and it is independent of the mergeModes, exactly as that section says.

Verified fix

One commit against 7b226da — internal/backends/dolt/bson_merge.go +63 −4
plus bson_merge_test.go +146. Same script, same fresh data dir:

--- NESTED: feature set meta.region, main set meta.owner ---
merge: {"commitId":"mb5vubjn37g4hq91s9cjdnjgbclc07l7", ... "ok":1}
doc after merge: {"_id":1,"meta":{"owner":"bob","region":"us"}}

go build ./..., go test ./internal/backends/dolt/ and
go test ./tests/ -run TestMergeMatrix all green on that one commit alone.
Happy to open it as a PR if useful.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions