drizzle-team / drizzle-team/drizzle-orm
[BUG]: commutativity check reports false conflicts for DDL inherited from a common ancestor (fan-in leaves)
- Dominant language
- TypeScript
- Stars
- 35.8k
- Forks
- 1.6k
- Avg merge
- 2d 7h
- Merged PRs (30d)
- 4
Description
### Report hasn't been filed before.
- [X] I have verified that the bug I'm about to report hasn't been filed before.
### What version of `drizzle-orm` are you using?
1.0.0-rc.5-ab785fc (also present in 1.0.0-rc.4)
### What version of `drizzle-kit` are you using?
1.0.0-rc.5-ab785fc (also present in 1.0.0-rc.4)
### Other packages
_No response_
### Describe the Bug
## `check` reports false conflicts for DDL that both leaves inherit from a common ancestor
When two leaf snapshots each list the **same set of multiple parents** — the fan-in `generate` writes when several leaves are open — `check` reports conflicts on DDL that both leaves inherit from a shared ancestor, and names objects that neither leaf's `migration.sql` touches.
The detector diffs `fork parent → leaf` and treats the result as that branch's own work. That reading holds when the fork parent is a true ancestor of both leaves. It breaks at a fan-in: a fan-in parent is by construction missing its siblings' work, and every leaf below the fan-in carries that work. The same inherited statement then appears on both sides and is reported as a collision.
This is distinct from #5639 (`create_index` footprint dropping the table name). Here footprint resolution is correct; the objects named are simply not the leaves' own work.
### Steps to reproduce
Save the script below as `make-repro.mjs`, then:
```bash
npm init -y
npm i drizzle-kit@1.0.0-rc.5-ab785fc drizzle-orm@1.0.0-rc.5-ab785fc
node make-repro.mjs && npx drizzle-kit check # FAILS
node make-repro.mjs --control && npx drizzle-kit check # PASSES
```
It writes hand-authored v8 snapshots directly, so no database and no `generate` run is needed.
DAG:
```
M0 ──┬── A: CREATE TABLE ta
└── B: CREATE TABLE tb
C = prevIds [A, B] + ALTER TABLE users ADD COLUMN c_only <- leaf 1
D = prevIds [A, B] + ALTER TABLE users ADD COLUMN d_only <- leaf 2
```
`C` and `D` overlap in nothing. `ta` and `tb` are each created exactly once, by `A` and `B`, and both of those are ancestors of both leaves.
make-repro.mjs
```js
import { mkdirSync, writeFileSync, rmSync } from "node:fs";
const control = process.argv.includes("--control");
const root = new URL(".", import.meta.url).pathname;
rmSync(`${root}/migrations`, { recursive: true, force: true });
const table = (name) => ({ isRlsEnabled: false, name, entityType: "tables", schema: "public" });
const column = (t, name) => ({
type: "integer", typeSchema: null, notNull: false, dimensions: 0,
default: null, generated: null, identity: null,
name, entityType: "columns", schema: "public", table: t,
});
const base = [table("users"), column("users", "id")];
const ta = [table("ta"), column("ta", "id")];
const tb = [table("tb"), column("tb", "id")];
const ID = (c) => `${c.repeat(8)}-${c.repeat(4)}-4${c.repeat(3)}-8${c.repeat(3)}-${c.repeat(12)}`;
const [M0, A, B, C, D] = ["0", "a", "b", "c", "d"].map(ID);
const snap = (id, prevIds, ddl) => ({ version: "8", dialect: "postgres", id, prevIds, ddl, renames: [] });
const write = (dir, snapshot, sql) => {
mkdirSync(`${root}/migrations/${dir}`, { recursive: true });
writeFileSync(`${root}/migrations/${dir}/snapshot.json`, JSON.stringify(snapshot, null, 1));
writeFileSync(`${root}/migrations/${dir}/migration.sql`, sql);
};
write("0001_base", snap(M0, [], base), `CREATE TABLE "users" ("id" integer);\n`);
if (control) {
// CONTROL - two leaves whose shared parent is a true ancestor of both. No fan-in.
write("0002_a", snap(A, [M0], [...base, column("users", "a_only")]), `ALTER TABLE "users" ADD COLUMN "a_only" integer;\n`);
write("0003_b", snap(B, [M0], [...base, column("users", "b_only")]), `ALTER TABLE "users" ADD COLUMN "b_only" integer;\n`);
} else {
write("0002_a", snap(A, [M0], [...base, ...ta]), `CREATE TABLE "ta" ("id" integer);\n`);
write("0003_b", snap(B, [M0], [...base, ...tb]), `CREATE TABLE "tb" ("id" integer);\n`);
write("0004_c", snap(C, [A, B], [...base, ...ta, ...tb, column("users", "c_only")]), `ALTER TABLE "users" ADD COLUMN "c_only" integer;\n`);
write("0005_d", snap(D, [A, B], [...base, ...ta, ...tb, column("users", "d_only")]), `ALTER TABLE "users" ADD COLUMN "d_only" integer;\n`);
}
mkdirSync(`${root}/src`, { recursive: true });
writeFileSync(`${root}/src/schema.ts`,
`import { pgTable, integer } from "drizzle-orm/pg-core";\nexport const users = pgTable("users", { id: integer("id") });\n`);
writeFileSync(`${root}/drizzle.config.ts`,
`import { defineConfig } from "drizzle-kit";\nexport default defineConfig({\n dialect: "postgresql",\n out: "./migrations",\n schema: "./src/schema.ts",\n dbCredentials: { url: "postgres://localhost:5432/unused" },\n});\n`);
console.log(control ? "wrote CONTROL case (expect: pass)" : "wrote REPRO case (expect: 2 bogus conflicts)");
```
### Actual result
```
Non-commutative migrations detected Found 2 conflicts across 2 migrations
migrations/0002_a
├── migrations/0004_c
│ └─ ⚠ create_table: tb in public schema
└── migrations/0005_d
└─ ⚠ create_table: tb in public schema
migrations/0003_b
├── migrations/0004_c
│ └─ ⚠ create_table: ta in public schema
└── migrations/0005_d
└─ ⚠ create_table: ta in public schema
```
### Desired result
`Everything's fine 🐶🔥` — the two leaves are commutative.
The `--control` case, which passes today, shows the boundary: two open leaves whose only shared parent is a true ancestor of both are handled correctly. So the defect is specific to the fan-in shape, not to having multiple leaves.
### Root cause
`src/dialects/postgres/commutativity.ts`, `detectNonCommutative` (bundled `bin.cjs:81751-81790` in rc.5). The mysql implementation has the same structure.
```js
for (const [prevId, childIds] of Object.entries(prevToChildren)) {
if (childIds.length <= 1) continue;
const parentSnapshot = parentNode ? parentNode.raw : drySnapshot;
...
const pending = diffOnce(parentSnapshot, leafNode.raw) // <-- "what this branch did"
...
const intersected = await this.getReasonsFromStatements(aStatements, bStatements, parentSnapshot);
```
`diffOnce(forkParent, leaf)` is everything the leaf has that the fork parent lacks. For a fan-in parent that includes its siblings' work, which every leaf below the fan-in also has. Nothing subtracts DDL reachable from an ancestor shared by both leaves.
The only guard is:
```js
if (groupB.some((leafId) => groupASet.has(leafId))) continue;
```
which skips a fork whose children reach a shared *leaf*. It does not cover a shared *ancestor*.
`lowestCommonAncestor` already exists (`bin.cjs:81663`) but is only used on the merge path (`bin.cjs:81805`), never in conflict detection.
### Suggested fix
Ask the question on the DAG instead of per fork. For each pair of live leaves `A`, `B`:
```
base = compose(maximal common ancestors of A and B) // composeMergeStatements, bin.cjs:81695
onlyA = diff(base → A)
onlyB = diff(base → B)
conflict <=> footprints(onlyA) ∩ footprints(onlyB) != ∅
```
Inherited DDL lands in `base`, so it cannot be reported. Genuinely duplicated work still lands in `onlyA` and `onlyB`, so real conflicts (same column with different types, drop-vs-alter on one column, two values appended to one enum, one index name over different columns) keep firing.
This should also be cheaper: cost scales with live leaf pairs rather than with every fork in history, and forks are permanent. It would additionally sidestep the path-enumeration blowup in #5960, since `collectLeaves` would no longer be on the hot path.
### Impact
- `generate` calls `checkHandler` first, so a false positive blocks migration generation for everyone on the project, not only CI.
- `--ignore-conflicts` is not a workaround. `checkHandler`'s return value *is* generate's diff base, so with the flag set generate either selects a converged ancestor by fewest-statements or returns `parentSnapshot: null` and falls back to the last snapshot in directory-name order. Both diff from the wrong state and can emit DDL that fails on deploy with "already exists".
- The only reliable workaround we found is to keep the history at a single leaf: merge the trunk, delete the new migration directory, and regenerate. On a project landing several migrations a day that means re-merging and regenerating whenever someone else's migration lands first.
### Related
- #5639 — a different commutativity false positive (`create_index` footprint), closed.
- #5960 — path-enumeration cost in the same detector, open.
- #5504, #5764 — merge-node reconstruction on the `generate` side.
Contributor guide
Research direction
Start in src/dialects/postgres/commutativity.ts at detectNonCommutative, then compare the MySQL implementation and the existing lowestCommonAncestor and composeMergeStatements paths. Run make-repro.mjs with both the failing and --control cases using drizzle-kit check. Done means fan-in leaves report no inherited conflicts while genuine conflicting changes still do.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- mysql, postgresql, typescript
- Domain
- databases, tooling
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100