Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 49 additions & 1 deletion src/odata-to-abstract-sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
FromNode,
WhereNode,
OrderByNode,
LockingClauseNode,

Check failure on line 24 in src/odata-to-abstract-sql.ts

View workflow job for this annotation

GitHub Actions / Flowzone / Test npm (v24.12.0)

Module '"@balena/abstract-sql-compiler"' has no exported member 'LockingClauseNode'.

Check failure on line 24 in src/odata-to-abstract-sql.ts

View workflow job for this annotation

GitHub Actions / Flowzone / Test npm (v20.19.6)

Module '"@balena/abstract-sql-compiler"' has no exported member 'LockingClauseNode'.

Check failure on line 24 in src/odata-to-abstract-sql.ts

View workflow job for this annotation

GitHub Actions / Flowzone / Test npm (v22.21.1)

Module '"@balena/abstract-sql-compiler"' has no exported member 'LockingClauseNode'.
LimitNode,
OffsetNode,
NumberTypeNodes,
Expand Down Expand Up @@ -231,7 +232,12 @@
public where: Array<WhereNode[1]> = [];
public joins: JoinTypeNodes[] = [];
public extras: Array<
FieldsNode | ValuesNode | OrderByNode | LimitNode | OffsetNode
| FieldsNode
| ValuesNode
| OrderByNode
| LimitNode
| OffsetNode
| LockingClauseNode
> = [];

merge(otherQuery: Query): void {
Expand Down Expand Up @@ -807,6 +813,48 @@
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,
Expand Down
32 changes: 20 additions & 12 deletions test/filterby.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof operandToAbstractSQLFactory>;

Expand Down Expand Up @@ -559,7 +560,8 @@ run(function () {
)
.from('pilot');
};
const updateWhere = [

const getUpdateWhere = (lockStrength: LockingClauseNode[1]) => [
'In',
['ReferencedField', 'pilot', 'id'],
[
Expand All @@ -569,6 +571,7 @@ run(function () {
pilotPilotCanFlyPlaneLeftJoin,
pilotPilotCanFlyPlanePlaneLeftJoin,
['Where', abstractsql],
['LockingClause', lockStrength, ['pilot']],
],
];

Expand Down Expand Up @@ -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'));
});
});

Expand Down Expand Up @@ -654,7 +657,7 @@ run(function () {
'Default',
)
.from('pilot')
.where(updateWhere);
.where(getUpdateWhere('UPDATE'));
});
});
});
Expand Down Expand Up @@ -706,6 +709,7 @@ run(function () {
],
],
['Where', abstractsql],
['LockingClause', 'UPDATE', ['pilot']],
],
]);
});
Expand Down Expand Up @@ -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]],
[
Expand All @@ -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'));
});
});

Expand Down Expand Up @@ -879,7 +885,7 @@ run([['Number', 1]], function () {
'Default',
)
.from('pilot')
.where(updateWhere);
.where(getUpdateWhere('UPDATE'));
});
});
});
Expand Down Expand Up @@ -2139,6 +2145,7 @@ test(
['Bind', 0],
],
],
['LockingClause', 'NO KEY UPDATE', ['copilot']],
],
],
],
Expand Down Expand Up @@ -2198,6 +2205,7 @@ test(
['Bind', 0],
],
],
['LockingClause', 'UPDATE', ['copilot']],
],
],
],
Expand Down
2 changes: 2 additions & 0 deletions test/paging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']],
],
]);
});
Expand All @@ -69,6 +70,7 @@ test('/pilot?$top=5&$skip=100', 'DELETE', (result) => {
['From', ['Table', 'pilot']],
['Limit', ['Bind', 0]],
['Offset', ['Bind', 1]],
['LockingClause', 'UPDATE', ['pilot']],
],
]);
});
Expand Down
Loading