Skip to content

Commit 1ca95df

Browse files
claude[bot]claude
andauthored
fix(driver-sql): the hash-shadow arm survives a plain unique over duplicate rows (#16288)
`syncDeclaredIndexes`'s `catch` handles a refused unique on two arms: the direct one, and the hash-shadow one MySQL takes when a key part exceeds the 768-char utf8mb4 ceiling. #14902 brought the direct arm to the ADR-0120 D4 disposition -- a uniqueness violation over existing rows is a durability degradation, not a fatal. The shadow arm still required a NULL-safe organization key part as well, so a PLAIN unique matched neither branch, fell through to the unkeyable-column refusal and took the boot down. Measured on live MySQL 8.0.46: the boot died with ER_BLOB_KEY_WITHOUT_LENGTH, advising a `maxLength` the field already declared, naming neither the two duplicate rows nor a remedy. Not a bare guard widening. The surviving branch's message says the rows violate the NULL-safe key and duplicate what the previous void constraint admitted (#5030); neither clause is true of a plain unique. The two arms are split so the NULL-safe one keeps its wording and the plain one carries the direct arm's reviewed sentence. Verified on live MySQL 8.0.46 through a new opt-in cell, with the two existing shadow cells as the firing control (green before and after). Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7a01847 commit 1ca95df

3 files changed

Lines changed: 315 additions & 0 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
MySQL: a plain unique index over existing duplicate rows no longer takes the boot down when the index has to be carried by a hash shadow.
6+
7+
`syncDeclaredIndexes` handles a declared unique that the database refuses in one `catch`, and that `catch` has two arms: the DIRECT one, and the hash-shadow one MySQL takes when a key part is wider than the 768-char utf8mb4 ceiling. #14902 taught the direct arm that a uniqueness violation over existing rows is a durability degradation rather than a fatal — log it, name the conflicting rows and the remedy, let the boot continue. The shadow arm kept the older guard, which also required a NULL-safe organization key part, so a PLAIN unique (`tenancy: { enabled: false }`, or an explicit `unique: 'global'`) matched neither branch.
8+
9+
Measured on live MySQL 8.0.46: the boot died carrying `ER_BLOB_KEY_WITHOUT_LENGTH` — a refusal about an unkeyable TEXT column, telling the operator to declare a `maxLength` the field already declared — while the real cause was two duplicate rows it never mentioned. It named no rows and no remedy.
10+
11+
The two arms now agree, and they say different things because they mean different things. The NULL-safe arm keeps its wording (existing rows violate the NULL-safe key, duplicating what the previous void constraint admitted); the plain arm gets the direct arm's reviewed sentence, because neither of those clauses is true of a plain unique — nothing admitted the rows, and there is no NULL-safe key. Widening the guard alone would have shipped a factually false durability log, which is worse than the throw it replaces.
12+
13+
`os migrate plan` already reported this operation as `destructive` with the row report and is unchanged.
Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #15479 — the HASH-SHADOW arm of `syncDeclaredIndexes`'s `catch`, on a PLAIN
5+
* unique over rows that already violate it.
6+
*
7+
* ## The defect
8+
*
9+
* #14902 (PR #15477) brought the DIRECT arm to parity: a plain unique over
10+
* existing duplicates logs on the durability channel and lets the boot
11+
* continue, instead of throwing the database's raw error. The hash-shadow arm
12+
* — one branch above it in the SAME `catch`, taken when MySQL refuses to key
13+
* the column directly — still asked `nullSafe.size > 0 && isUniqueViolation…`,
14+
* so with no organization key part the branch did not fire, the code fell
15+
* through to `logDurabilityFailure(unkeyable, msg)` + `throw`, and the boot
16+
* died carrying a message about an UNKEYABLE TEXT COLUMN while the actual
17+
* cause was duplicate rows — naming neither the conflicting rows nor a remedy.
18+
*
19+
* ## Why this file is a live cell and could not be a SQLite one
20+
*
21+
* The shadow route exists only because MySQL refuses a key part over the
22+
* 768-char utf8mb4 ceiling; SQLite and Postgres never refuse, so they never
23+
* reach this arm. `maxLength: 1024` is what selects it. The card that filed
24+
* this said the branch was unreachable in the dispatch container — measured
25+
* false: MySQL 8.0 installs from the distro archive and this suite drives it.
26+
*
27+
* ## The two-arm message split, which is the half that is NOT a guard widening
28+
*
29+
* The surviving branch's message said "existing rows violate the NULL-safe key
30+
* (duplicates the previous void constraint admitted, #5030)". Neither clause is
31+
* true of a plain unique: nothing admitted those rows, and there is no NULL-safe
32+
* key. Widening the guard and leaving one message would ship a FACTUALLY FALSE
33+
* durability log — worse than the throw it replaces, because it sends the
34+
* operator hunting for a NULL-distinct index that was never there. So the
35+
* NULL-safe branch keeps its wording (asserted below as the CONTROL) and the
36+
* plain branch gets the direct arm's reviewed sentence.
37+
*
38+
* Opt-in, like every live cell in this package:
39+
*
40+
* OS_TEST_MYSQL_URL=mysql://root:root@127.0.0.1:3306/conformance \
41+
* pnpm --filter @objectstack/driver-sql test
42+
*/
43+
44+
import { describe, it, expect, afterEach } from 'vitest';
45+
import { SqlDriver } from '../src/index.js';
46+
import { isHashShadowColumn } from './schema-drift.js';
47+
import { MYSQL_CELL, declareDialectCell } from './live-dialect-matrix.testkit.js';
48+
49+
/** 900 chars — comfortably over the 768-char keyable ceiling, so `v` is TEXT. */
50+
const LONG = 'p'.repeat(900);
51+
52+
/**
53+
* A tenancy-DISABLED object whose one long text field carries a PLAIN unique.
54+
* `maxLength: 1024` exceeds the keyable ceiling, so MySQL refuses the direct
55+
* index and the sync takes the shadow route; `tenancy: { enabled: false }` is
56+
* one of the two shapes #14902 identified as reaching the plain path (the other
57+
* is an explicit `unique: 'global'`, exercised by `globalUniqueOn` below).
58+
*/
59+
const plainUniqueOn = (name: string) => ({
60+
name,
61+
tenancy: { enabled: false },
62+
fields: { v: { type: 'text', maxLength: 1024 } },
63+
indexes: [{ fields: ['v'], unique: true as const, name: `uniq_${name}_v` }],
64+
});
65+
66+
/** The second shape of the same path: tenanted object, explicit global scope. */
67+
const globalUniqueOn = (name: string) => ({
68+
name,
69+
fields: {
70+
organization_id: { type: 'string' },
71+
v: { type: 'text', maxLength: 1024 },
72+
},
73+
indexes: [{ fields: ['v'], unique: 'global' as const, name: `uniq_${name}_v` }],
74+
});
75+
76+
/** The CONTROL shape: an organization-scoped unique, i.e. the NULL-safe arm. */
77+
const orgUniqueOn = (name: string) => ({
78+
name,
79+
fields: {
80+
organization_id: { type: 'string' },
81+
v: { type: 'text', maxLength: 1024 },
82+
},
83+
indexes: [{ fields: ['v'], unique: 'organization' as const, name: `uniq_${name}_v` }],
84+
});
85+
86+
declareDialectCell(MYSQL_CELL, 'hash-shadow plain unique over duplicates (#15479)', (cell) => {
87+
describe('hash-shadow arm, plain unique over duplicate rows on live MySQL (#15479)', () => {
88+
let driver: SqlDriver;
89+
afterEach(async () => {
90+
await driver?.disconnect().catch(() => {});
91+
});
92+
93+
/** Physical truth, read back from the catalog rather than from our DDL. */
94+
const catalog = async (table: string) => {
95+
const knex = (driver as any).knex;
96+
const cols = await knex
97+
.select('COLUMN_NAME', 'DATA_TYPE', 'GENERATION_EXPRESSION')
98+
.from('information_schema.COLUMNS')
99+
.where({ TABLE_SCHEMA: knex.client.database(), TABLE_NAME: table });
100+
const idx = await knex
101+
.select('INDEX_NAME', 'NON_UNIQUE', 'COLUMN_NAME', 'SUB_PART')
102+
.from('information_schema.STATISTICS')
103+
.where({ TABLE_SCHEMA: knex.client.database(), TABLE_NAME: table });
104+
return { cols, idx };
105+
};
106+
107+
/** Collect this driver's log lines rather than letting them reach stdout. */
108+
const spy = (): string[] => {
109+
const logs: string[] = [];
110+
(driver as any).logger = {
111+
warn: (msg: string) => logs.push(String(msg)),
112+
info: (msg: string) => logs.push(String(msg)),
113+
error: (msg: string) => logs.push(String(msg)),
114+
};
115+
return logs;
116+
};
117+
118+
/**
119+
* Boot the object WITHOUT its unique index, accumulate two rows that
120+
* violate it, then re-register WITH the index — the legacy-upgrade shape.
121+
*/
122+
const seedDuplicatesThenDeclare = async (
123+
meta: { name: string; indexes: unknown[] },
124+
row: Record<string, unknown>,
125+
): Promise<{ logs: string[]; err: unknown }> => {
126+
driver = new SqlDriver(cell.config());
127+
const logs = spy();
128+
await driver.initObjects([{ ...meta, indexes: [] }] as any);
129+
const knex = (driver as any).knex;
130+
await knex(meta.name).insert([
131+
{ id: 'a', ...row },
132+
{ id: 'b', ...row },
133+
]);
134+
const err: unknown = await driver.initObjects([meta] as any).then(
135+
() => null,
136+
(e) => e,
137+
);
138+
return { logs, err };
139+
};
140+
141+
// ── Why each it() carries an explicit 60_000 budget (#13902) ──
142+
// Each block constructs a FRESH `new SqlDriver(...)` against this cell's
143+
// live server inside its own body, so the connect cycle plus the schema-sync
144+
// DDL and catalog read-back are paid PER TEST rather than once in a
145+
// beforeAll. Sized like this package's siblings; not a claim that these are
146+
// normally anywhere near that slow.
147+
148+
/**
149+
* THE CARD'S THREE ACCEPTANCE CONDITIONS, on the `tenancy: { enabled: false }`
150+
* shape: the boot survives, the durability log names the conflicting group
151+
* and the remedy, and the index is absent afterwards.
152+
*/
153+
it('survives the boot and diagnoses duplicates under a tenancy-disabled plain unique', async () => {
154+
const { logs, err } = await seedDuplicatesThenDeclare(plainUniqueOn('os15479_plain'), {
155+
v: LONG,
156+
});
157+
158+
expect(err, 'the boot must NOT die: this is a degradation, not a fatal').toBeNull();
159+
160+
const diagnosis = logs.find((l) => l.includes("cannot create hash-shadow unique index 'uniq_os15479_plain_v'"));
161+
expect(diagnosis, 'the degradation must reach the durability channel').toBeTruthy();
162+
expect(diagnosis).toMatch(/Conflicting group\(s\): \(v="p+"\) × 2 rows/);
163+
expect(diagnosis).toMatch(/os migrate plan/);
164+
expect(diagnosis).toContain("The constraint 'v' is NOT enforced");
165+
166+
// ⛔ And it must NOT carry the NULL-safe arm's framing, which is false
167+
// here: nothing admitted these rows and there is no NULL-safe key.
168+
expect(diagnosis).not.toContain('#5030');
169+
expect(diagnosis).not.toContain('NULL-safe');
170+
expect(diagnosis).not.toContain('COALESCE');
171+
172+
// The constraint is honestly ABSENT, and the atomic ALTER left no
173+
// orphaned shadow column behind.
174+
const { cols, idx } = await catalog('os15479_plain');
175+
expect(idx.some((i: any) => i.INDEX_NAME === 'uniq_os15479_plain_v')).toBe(false);
176+
expect(cols.filter((c: any) => isHashShadowColumn(c.COLUMN_NAME))).toEqual([]);
177+
}, 60_000);
178+
179+
/**
180+
* The same disposition on the OTHER shape that reaches the plain path — an
181+
* explicit `unique: 'global'` on a tenanted object. Two shapes because
182+
* #14902 measured both, and a guard keyed on the wrong one would pass here
183+
* and fail in production.
184+
*/
185+
it("survives the boot under an explicit unique: 'global' on a tenanted object", async () => {
186+
const { logs, err } = await seedDuplicatesThenDeclare(globalUniqueOn('os15479_global'), {
187+
v: LONG,
188+
organization_id: 'org_a',
189+
});
190+
191+
expect(err, 'the boot must NOT die').toBeNull();
192+
const diagnosis = logs.find((l) => l.includes("cannot create hash-shadow unique index 'uniq_os15479_global_v'"));
193+
expect(diagnosis, 'the degradation must reach the durability channel').toBeTruthy();
194+
expect(diagnosis).toMatch(/Conflicting group\(s\):/);
195+
expect(diagnosis).not.toContain('#5030');
196+
expect(diagnosis).not.toContain('COALESCE');
197+
198+
const { idx } = await catalog('os15479_global');
199+
expect(idx.some((i: any) => i.INDEX_NAME === 'uniq_os15479_global_v')).toBe(false);
200+
}, 60_000);
201+
202+
/**
203+
* ⛔ THE CONTROL for the message split: the NULL-safe arm keeps its OWN
204+
* wording. A one-character fix that widened the guard and left a single
205+
* message would pass the two blocks above and fail exactly here — and the
206+
* inverse mistake, rewriting both arms into the plain sentence, fails here
207+
* too.
208+
*/
209+
it('leaves the NULL-safe arm saying the NULL-safe thing', async () => {
210+
const { logs, err } = await seedDuplicatesThenDeclare(orgUniqueOn('os15479_org'), {
211+
v: LONG,
212+
organization_id: null,
213+
});
214+
215+
expect(err, 'the NULL-safe arm already survived the boot').toBeNull();
216+
const diagnosis = logs.find((l) => l.includes("cannot create hash-shadow unique index 'uniq_os15479_org_v'"));
217+
expect(diagnosis, 'the NULL-safe degradation must still be logged').toBeTruthy();
218+
expect(diagnosis).toContain('#5030');
219+
expect(diagnosis).toContain('NULL-safe');
220+
expect(diagnosis).toContain("COALESCE(organization_id, '__global__')");
221+
}, 60_000);
222+
223+
/**
224+
* The positive control that the widened guard did not turn the shadow route
225+
* off: over CLEAN data the plain unique is still CREATED, on the shadow
226+
* column, and still enforces.
227+
*/
228+
it('still creates and enforces the plain shadow unique over clean data', async () => {
229+
driver = new SqlDriver(cell.config());
230+
spy();
231+
await driver.initObjects([plainUniqueOn('os15479_clean')] as any);
232+
233+
const { cols, idx } = await catalog('os15479_clean');
234+
const shadow = cols.find((c: any) => isHashShadowColumn(c.COLUMN_NAME));
235+
expect(shadow, 'a shadow column must exist').toBeTruthy();
236+
const carried = idx.filter((i: any) => isHashShadowColumn(i.COLUMN_NAME));
237+
expect(carried.length).toBe(1);
238+
expect(Number(carried[0].NON_UNIQUE)).toBe(0);
239+
240+
const knex = (driver as any).knex;
241+
await knex('os15479_clean').insert({ id: 'a', v: LONG });
242+
await expect(knex('os15479_clean').insert({ id: 'b', v: LONG })).rejects.toThrow(/duplicate/i);
243+
await knex('os15479_clean').insert({ id: 'c', v: 'q'.repeat(900) });
244+
}, 60_000);
245+
});
246+
});

packages/drivers/driver-sql/src/sql-driver.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11816,6 +11816,12 @@ export class SqlDriver implements IDataDriver {
1181611816
* data that already violates it gets the SAME disposition, where it used to
1181711817
* throw the database's raw error and take the boot down naming no rows and
1181811818
* no remedy.
11819+
* - #15479: the same disposition on the HASH-SHADOW route (the MySQL arm
11820+
* #11627 added, taken when the server refuses to key the column directly).
11821+
* Its guard asked `nullSafe.size > 0` as well, so a plain unique reached
11822+
* neither branch and took the boot down carrying the UNKEYABLE-COLUMN
11823+
* refusal instead of the duplicate rows. Measured on live MySQL 8.0.46 in
11824+
* `sql-driver-15479-shadow-plain-unique-duplicates.test.ts`.
1181911825
*/
1182011826
protected async syncDeclaredIndexes(
1182111827
tableName: string,
@@ -11954,6 +11960,56 @@ export class SqlDriver implements IDataDriver {
1195411960
);
1195511961
continue;
1195611962
}
11963+
if (isUniqueViolationError(shadowErr)) {
11964+
// #15479 — the PLAIN unique on the SHADOW route: no
11965+
// organization key part at all (`tenancy: { enabled: false }`
11966+
// or an explicit `unique: 'global'`), reached here because
11967+
// MySQL refused to key the column directly. The uniqueness
11968+
// limb is supplied by the enclosing `if (unique)` a few lines
11969+
// above, which is why this asks only the violation question --
11970+
// the same load-bearing role the explicit `unique &&` limb
11971+
// plays on the direct arm below, where no enclosing block
11972+
// supplies it.
11973+
//
11974+
// This branch used to be part of the NULL-safe one above,
11975+
// behind `nullSafe.size > 0`, so with no organization key part
11976+
// NOTHING fired: the code fell through to
11977+
// `logDurabilityFailure(unkeyable, msg)` + `throw` and the boot
11978+
// DIED carrying `ER_BLOB_KEY_WITHOUT_LENGTH` -- a message about
11979+
// an unkeyable TEXT column, telling the operator to declare a
11980+
// `maxLength` the field already declares, naming NO rows and NO
11981+
// remedy -- while the actual cause was duplicate rows. #14902
11982+
// fixed exactly that on the direct arm; the two arms of one
11983+
// `catch` then disagreed about one question.
11984+
//
11985+
// The message is deliberately NOT the NULL-safe one above.
11986+
// Neither of its clauses is true here: nothing "admitted" these
11987+
// rows (there was no previous void constraint, #5030), and
11988+
// there is no NULL-safe key. Widening the guard while keeping
11989+
// one message would ship a factually FALSE durability log --
11990+
// worse than the throw it replaces, because it sends the
11991+
// operator hunting a NULL-distinct index that never existed.
11992+
// The sentence below is the direct arm's, verbatim; only the
11993+
// route noun ("hash-shadow") is this arm's own, because it is
11994+
// what honestly names which physical route was attempted, and
11995+
// the NULL-safe branch above already spells it that way.
11996+
let report = '';
11997+
try {
11998+
const duplicates = await this.probeNullSafeUniqueDuplicates(tableName, columns, []);
11999+
if (duplicates.length > 0) {
12000+
report = ` Conflicting group(s): ${formatDuplicateGroups(duplicates)}.`;
12001+
}
12002+
} catch {
12003+
// The probe is a diagnostic; the report below stands without it.
12004+
}
12005+
this.logDurabilityFailure(
12006+
`[sql-driver] cannot create hash-shadow unique index '${name}' on "${tableName}" — existing ` +
12007+
`rows violate it.${report} The constraint '${columns.join(', ')}' is NOT enforced until the ` +
12008+
`data is deduplicated: run "os migrate plan" for the conflicting rows.`,
12009+
shadowMsg,
12010+
);
12011+
continue;
12012+
}
1195712013
// Fall through to the named refusal, which is still the honest
1195812014
// outcome — but say that the shadow route was tried and why it
1195912015
// did not land, so this does not read as never having been

0 commit comments

Comments
 (0)