Skip to content

Commit 9982385

Browse files
committed
fix(sql-orm-client): scope single-row mutations by full row identity
The previous fix scoped update()/delete() by `resolvePrimaryKeyColumn`, which only returns the first PK column. Composite primary keys would collapse to a single column and fail to identify a row uniquely, and PK-less tables (with a unique constraint instead) had no path at all. Introduce `resolveRowIdentityColumns`: PK columns if present, else the first unique constraint, else throw. The lookup helper now projects all identity columns and builds a multi-column WHERE criterion via `shorthandToWhereExpr`, which produces the AND-of-equalities filter that uniquely identifies the row. Also wrap the SELECT-then-mutate pair in `withMutationScope` so both queries run against the same transaction-bound scope, matching what `executeNestedUpdateMutation` already does for the nested-callback path. A new `#withRuntime` helper clones the collection bound to the scoped runtime so the existing Collection-level helpers (`first`, `updateAll`, `#executeDeleteReturning`) continue to work inside the callback.
1 parent 06f994e commit 9982385

3 files changed

Lines changed: 137 additions & 27 deletions

File tree

packages/3-extensions/sql-orm-client/src/collection-contract.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,25 @@ export function resolvePrimaryKeyColumn(contract: Contract<SqlStorage>, tableNam
322322
return contract.storage.tables[tableName]?.primaryKey?.columns[0] ?? 'id';
323323
}
324324

325+
export function resolveRowIdentityColumns(
326+
contract: Contract<SqlStorage>,
327+
tableName: string,
328+
): readonly string[] {
329+
const table = contract.storage.tables[tableName];
330+
if (!table) {
331+
return [];
332+
}
333+
if (table.primaryKey && table.primaryKey.columns.length > 0) {
334+
return table.primaryKey.columns;
335+
}
336+
for (const unique of table.uniques) {
337+
if (unique.columns.length > 0) {
338+
return unique.columns;
339+
}
340+
}
341+
return [];
342+
}
343+
325344
export function assertReturningCapability(contract: Contract<SqlStorage>, action: string): void {
326345
if (hasContractCapability(contract, 'returning')) {
327346
return;

packages/3-extensions/sql-orm-client/src/collection.ts

Lines changed: 60 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
resolveModelTableName,
2727
resolvePolymorphismInfo,
2828
resolvePrimaryKeyColumn,
29+
resolveRowIdentityColumns,
2930
resolveUpsertConflictColumns,
3031
} from './collection-contract';
3132
import { dispatchCollectionRows } from './collection-dispatch';
@@ -108,6 +109,7 @@ import {
108109
type RelatedModelName,
109110
type RelationNames,
110111
type ResolvedCreateInput,
112+
type RuntimeQueryable,
111113
type ShorthandWhereFilter,
112114
type UniqueConstraintCriterion,
113115
type VariantModelRow,
@@ -1063,17 +1065,20 @@ export class Collection<
10631065
return this.#reloadMutationRowByPrimaryKey(pkCriterion);
10641066
}
10651067

1066-
const pkWhere = await this.#findFirstMatchingPkWhere();
1067-
if (!pkWhere) {
1068-
return null;
1069-
}
1070-
const narrowed = this.#clone({ filters: [pkWhere] });
1071-
const rows = await narrowed.updateAll(
1072-
data as State['hasWhere'] extends true
1073-
? Partial<DefaultModelRow<TContract, ModelName>>
1074-
: never,
1075-
);
1076-
return rows[0] ?? null;
1068+
return withMutationScope(this.ctx.runtime, async (scope) => {
1069+
const scoped = this.#withRuntime(scope);
1070+
const identityWhere = await scoped.#findFirstMatchingRowIdentityWhere();
1071+
if (!identityWhere) {
1072+
return null;
1073+
}
1074+
const narrowed = scoped.#clone({ filters: [identityWhere] });
1075+
const rows = await narrowed.updateAll(
1076+
data as State['hasWhere'] extends true
1077+
? Partial<DefaultModelRow<TContract, ModelName>>
1078+
: never,
1079+
);
1080+
return rows[0] ?? null;
1081+
});
10771082
}
10781083

10791084
updateAll(
@@ -1149,13 +1154,16 @@ export class Collection<
11491154
this: State['hasWhere'] extends true ? Collection<TContract, ModelName, Row, State> : never,
11501155
): Promise<Row | null> {
11511156
assertReturningCapability(this.contract, 'delete()');
1152-
const pkWhere = await this.#findFirstMatchingPkWhere();
1153-
if (!pkWhere) {
1154-
return null;
1155-
}
1156-
const narrowed = this.#clone({ filters: [pkWhere] });
1157-
const rows = await narrowed.#executeDeleteReturning().toArray();
1158-
return rows[0] ?? null;
1157+
return withMutationScope(this.ctx.runtime, async (scope) => {
1158+
const scoped = this.#withRuntime(scope);
1159+
const identityWhere = await scoped.#findFirstMatchingRowIdentityWhere();
1160+
if (!identityWhere) {
1161+
return null;
1162+
}
1163+
const narrowed = scoped.#clone({ filters: [identityWhere] });
1164+
const rows = await narrowed.#executeDeleteReturning().toArray();
1165+
return rows[0] ?? null;
1166+
});
11591167
}
11601168

11611169
deleteAll(
@@ -1230,22 +1238,37 @@ export class Collection<
12301238
return criterion;
12311239
}
12321240

1233-
async #findFirstMatchingPkWhere(): Promise<AnyExpression | null> {
1234-
const pkColumn = resolvePrimaryKeyColumn(this.contract, this.tableName);
1235-
const firstRow = await this.#clone({ selectedFields: [pkColumn], includes: [] }).first();
1241+
async #findFirstMatchingRowIdentityWhere(): Promise<AnyExpression | null> {
1242+
const identityColumns = resolveRowIdentityColumns(this.contract, this.tableName);
1243+
if (identityColumns.length === 0) {
1244+
throw new Error(
1245+
`update()/delete() on model "${this.modelName}" requires the table to have a primary key or unique constraint`,
1246+
);
1247+
}
1248+
const firstRow = await this.#clone({
1249+
selectedFields: [...identityColumns],
1250+
includes: [],
1251+
}).first();
12361252
if (!firstRow) {
12371253
return null;
12381254
}
1239-
const pkCriterion = buildPrimaryKeyFilterFromRow(
1240-
this.contract,
1241-
this.modelName,
1242-
firstRow as Record<string, unknown>,
1243-
);
1255+
const columnToField = getColumnToFieldMap(this.contract, this.modelName);
1256+
const criterion: Record<string, unknown> = {};
1257+
for (const column of identityColumns) {
1258+
const fieldName = columnToField[column] ?? column;
1259+
const value = (firstRow as Record<string, unknown>)[fieldName];
1260+
if (value === undefined) {
1261+
throw new Error(
1262+
`Missing identity field "${fieldName}" while resolving single-row scope for model "${this.modelName}"`,
1263+
);
1264+
}
1265+
criterion[fieldName] = value;
1266+
}
12441267
return (
12451268
shorthandToWhereExpr(
12461269
this.ctx.context,
12471270
this.modelName,
1248-
pkCriterion as ShorthandWhereFilter<TContract, ModelName>,
1271+
criterion as ShorthandWhereFilter<TContract, ModelName>,
12491272
) ?? null
12501273
);
12511274
}
@@ -1304,6 +1327,16 @@ export class Collection<
13041327
});
13051328
}
13061329

1330+
#withRuntime(runtime: RuntimeQueryable): Collection<TContract, ModelName, Row, State> {
1331+
const Ctor = this.constructor as CollectionConstructor<TContract>;
1332+
return new Ctor({ ...this.ctx, runtime }, this.modelName, {
1333+
tableName: this.tableName,
1334+
state: this.state,
1335+
registry: this.registry,
1336+
includeRefinementMode: this.includeRefinementMode,
1337+
}) as unknown as Collection<TContract, ModelName, Row, State>;
1338+
}
1339+
13071340
#cloneWithRow<NextRow, NextState extends CollectionTypeState = State>(
13081341
overrides: Partial<CollectionState>,
13091342
): Collection<TContract, ModelName, NextRow, NextState> {

packages/3-extensions/sql-orm-client/test/collection-contract.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
resolveModelTableName,
88
resolvePolymorphismInfo,
99
resolvePrimaryKeyColumn,
10+
resolveRowIdentityColumns,
1011
resolveUpsertConflictColumns,
1112
} from '../src/collection-contract';
1213
import { buildMixedPolyContract, getTestContract } from './helpers';
@@ -235,6 +236,63 @@ describe('collection-contract capability detection', () => {
235236
expect(isToOneCardinality('M:N')).toBe(false);
236237
expect(isToOneCardinality(undefined)).toBe(false);
237238
});
239+
240+
describe('resolveRowIdentityColumns()', () => {
241+
const buildContract = (table: {
242+
primaryKey?: { columns: readonly string[] };
243+
uniques?: ReadonlyArray<{ columns: readonly string[] }>;
244+
}) =>
245+
({
246+
storage: {
247+
tables: {
248+
t: {
249+
primaryKey: table.primaryKey,
250+
uniques: table.uniques ?? [],
251+
},
252+
},
253+
},
254+
}) as unknown as Parameters<typeof resolveRowIdentityColumns>[0];
255+
256+
it('returns primary key columns when present', () => {
257+
expect(
258+
resolveRowIdentityColumns(buildContract({ primaryKey: { columns: ['id'] } }), 't'),
259+
).toEqual(['id']);
260+
});
261+
262+
it('returns composite primary key columns when present', () => {
263+
expect(
264+
resolveRowIdentityColumns(buildContract({ primaryKey: { columns: ['a', 'b'] } }), 't'),
265+
).toEqual(['a', 'b']);
266+
});
267+
268+
it('falls back to first unique constraint when no primary key', () => {
269+
expect(
270+
resolveRowIdentityColumns(
271+
buildContract({ uniques: [{ columns: ['email'] }, { columns: ['handle'] }] }),
272+
't',
273+
),
274+
).toEqual(['email']);
275+
});
276+
277+
it('returns composite unique columns when no primary key', () => {
278+
expect(
279+
resolveRowIdentityColumns(
280+
buildContract({ uniques: [{ columns: ['tenant_id', 'slug'] }] }),
281+
't',
282+
),
283+
).toEqual(['tenant_id', 'slug']);
284+
});
285+
286+
it('returns empty array when neither primary key nor uniques are defined', () => {
287+
expect(resolveRowIdentityColumns(buildContract({}), 't')).toEqual([]);
288+
});
289+
290+
it('returns empty array for unknown tables', () => {
291+
expect(
292+
resolveRowIdentityColumns(buildContract({ primaryKey: { columns: ['id'] } }), 'missing'),
293+
).toEqual([]);
294+
});
295+
});
238296
});
239297

240298
describe('resolvePolymorphismInfo()', () => {

0 commit comments

Comments
 (0)