drizzle-team / drizzle-team/drizzle-orm

[BUG]: commutativity check reports false conflicts for DDL inherited from a common ancestor (fan-in leaves)

Open
#6,216 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.