diff --git a/src/odata-to-abstract-sql.ts b/src/odata-to-abstract-sql.ts index c5f16c7..992b951 100644 --- a/src/odata-to-abstract-sql.ts +++ b/src/odata-to-abstract-sql.ts @@ -21,6 +21,7 @@ import type { FromNode, WhereNode, OrderByNode, + LockingClauseNode, LimitNode, OffsetNode, NumberTypeNodes, @@ -231,7 +232,12 @@ class Query { public where: Array = []; public joins: JoinTypeNodes[] = []; public extras: Array< - FieldsNode | ValuesNode | OrderByNode | LimitNode | OffsetNode + | FieldsNode + | ValuesNode + | OrderByNode + | LimitNode + | OffsetNode + | LockingClauseNode > = []; merge(otherQuery: Query): void { @@ -807,6 +813,48 @@ export class OData2AbstractSQL { if (hasQueryOpts) { this.AddQueryOptions(resource, path, subQuery); } + + let lockStrength: LockingClauseNode[1] = 'UPDATE'; + if ( + method === 'PATCH' && + // That's always set for PATCHes, so we only check it to keep TS happy. + bindVars != null + ) { + // The FOR UPDATE lock mode is also acquired by any DELETE on a row, + // and also by an UPDATE that modifies the values of certain columns. + // Currently, the set of columns considered for the UPDATE case are those + // that have a unique index on them that can be used in a foreign key + // (so partial indexes and expressional indexes are not considered). + // FOR NO KEY UPDATE ... is acquired by any UPDATE that does not acquire a FOR UPDATE lock. + // See: https://www.postgresql.org/docs/17/explicit-locking.html#LOCKING-ROWS + const fieldsPartOfUniqueIndexes = new Set([ + ...resource.fields + .filter( + (field) => + field.index === 'UNIQUE' || field.index === 'PRIMARY KEY', + ) + .map((field) => field.fieldName), + ...resource.indexes + .filter( + (idx) => + (idx.type === 'UNIQUE' && idx.predicate == null) || + idx.type === 'PRIMARY KEY', + ) + .flatMap((idx) => idx.fields), + ]); + const updatesFieldPartOfUniqueIndex = bindVars.some((b) => + fieldsPartOfUniqueIndexes.has(b[0]), + ); + if (!updatesFieldPartOfUniqueIndex) { + lockStrength = 'NO KEY UPDATE'; + } + } + subQuery.extras.push([ + 'LockingClause', + lockStrength, + [resource.tableAlias], + ]); + query.where.push([ 'In', referencedIdField, diff --git a/test/filterby.ts b/test/filterby.ts index b257d20..da7243b 100644 --- a/test/filterby.ts +++ b/test/filterby.ts @@ -15,6 +15,7 @@ let operandToAbstractSQLFactory = $operandToAbstractSQLFactory; import test from './test.js'; import _ from 'lodash'; +import type { LockingClauseNode } from '@balena/abstract-sql-compiler'; let operandToAbstractSQL: ReturnType; @@ -559,7 +560,8 @@ run(function () { ) .from('pilot'); }; - const updateWhere = [ + + const getUpdateWhere = (lockStrength: LockingClauseNode[1]) => [ 'In', ['ReferencedField', 'pilot', 'id'], [ @@ -569,6 +571,7 @@ run(function () { pilotPilotCanFlyPlaneLeftJoin, pilotPilotCanFlyPlanePlaneLeftJoin, ['Where', abstractsql], + ['LockingClause', lockStrength, ['pilot']], ], ]; @@ -605,7 +608,7 @@ run(function () { .to.be.a.query.that.updates.fields('name') .values(['Bind', ['pilot', 'name']]) .from('pilot') - .where(updateWhere); + .where(getUpdateWhere('NO KEY UPDATE')); }); }); @@ -654,7 +657,7 @@ run(function () { 'Default', ) .from('pilot') - .where(updateWhere); + .where(getUpdateWhere('UPDATE')); }); }); }); @@ -706,6 +709,7 @@ run(function () { ], ], ['Where', abstractsql], + ['LockingClause', 'UPDATE', ['pilot']], ], ]); }); @@ -806,7 +810,14 @@ run([['Number', 1]], function () { ) .from('pilot'); }; - const updateWhere = [ + + test('/pilot(1)?$filter=' + odata, 'POST', { name }, (result) => { + it('should insert into pilot where "' + odata + '"', () => { + insertTest(result); + }); + }); + + const getUpdateWhere = (lockStrength: LockingClauseNode[1]) => [ 'And', ['IsNotDistinctFrom', ['ReferencedField', 'pilot', 'id'], ['Bind', 0]], [ @@ -820,23 +831,18 @@ run([['Number', 1]], function () { ], ['From', ['Table', 'pilot']], ['Where', abstractsql], + ['LockingClause', lockStrength, ['pilot']], ], ], ]; - test('/pilot(1)?$filter=' + odata, 'POST', { name }, (result) => { - it('should insert into pilot where "' + odata + '"', () => { - insertTest(result); - }); - }); - test('/pilot(1)?$filter=' + odata, 'PATCH', { name }, (result) => { it('should update the pilot with id 1', () => { expect(result) .to.be.a.query.that.updates.fields('name') .values(['Bind', ['pilot', 'name']]) .from('pilot') - .where(updateWhere); + .where(getUpdateWhere('NO KEY UPDATE')); }); }); @@ -879,7 +885,7 @@ run([['Number', 1]], function () { 'Default', ) .from('pilot') - .where(updateWhere); + .where(getUpdateWhere('UPDATE')); }); }); }); @@ -2139,6 +2145,7 @@ test( ['Bind', 0], ], ], + ['LockingClause', 'NO KEY UPDATE', ['copilot']], ], ], ], @@ -2198,6 +2205,7 @@ test( ['Bind', 0], ], ], + ['LockingClause', 'UPDATE', ['copilot']], ], ], ], diff --git a/test/paging.ts b/test/paging.ts index ad43d04..259a032 100644 --- a/test/paging.ts +++ b/test/paging.ts @@ -49,6 +49,7 @@ test('/pilot?$top=5&$skip=100', 'PATCH', { name }, (result) => { ['From', ['Table', 'pilot']], ['Limit', ['Bind', 0]], ['Offset', ['Bind', 1]], + ['LockingClause', 'NO KEY UPDATE', ['pilot']], ], ]); }); @@ -69,6 +70,7 @@ test('/pilot?$top=5&$skip=100', 'DELETE', (result) => { ['From', ['Table', 'pilot']], ['Limit', ['Bind', 0]], ['Offset', ['Bind', 1]], + ['LockingClause', 'UPDATE', ['pilot']], ], ]); });