Skip to content

Commit 3f64763

Browse files
patricebenderpatricebender
authored
chore: re-write filter at from.at(-1) into queries where (#1373)
this is a preparation for #1361 and it also makes our lifes easier as the infix filter at this place is semantically equivalent to a regular `where` clause.
1 parent 39e8a99 commit 3f64763

File tree

7 files changed

+65
-61
lines changed

7 files changed

+65
-61
lines changed

‎db-service/lib/cqn4sql.js‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ function cqn4sql(originalQuery, model) {
7474
if (inferred.SELECT?.from.ref?.at(-1).id) {
7575
assignQueryModifiers(inferred.SELECT, inferred.SELECT.from.ref.at(-1))
7676
}
77+
if (inferred.DELETE?.from.ref?.at(-1).id) {
78+
assignQueryModifiers(inferred.DELETE, inferred.DELETE.from.ref.at(-1))
79+
}
80+
if (inferred.UPDATE?.entity.ref?.at(-1).id) {
81+
assignQueryModifiers(inferred.UPDATE, inferred.UPDATE.entity.ref.at(-1))
82+
}
7783
inferred = infer(inferred, model)
7884
const { getLocalizedName, isLocalized, getDefinition } = getModelUtils(model, originalQuery) // TODO: pass model to getModelUtils
7985
// if the query has custom joins we don't want to transform it
@@ -1460,24 +1466,12 @@ function cqn4sql(originalQuery, model) {
14601466
else transformedTokenStream.push({ list: getTransformedTokenStream(list, { $baseLink, prop: 'list' }) })
14611467
}
14621468
} else if (tokenStream.length === 1 && token.val && $baseLink) {
1463-
// infix filter - OData variant w/o mentioning key --> flatten out and compare each leaf to token.val
1469+
// infix filter - OData variant w/o mentioning key
14641470
const def = getDefinition($baseLink.definition.target) || $baseLink.definition
1465-
const keys = def.keys // use key aspect on entity
1466-
const keyValComparisons = []
1467-
const flatKeys = []
1468-
for (const v of Object.values(keys)) {
1469-
if (v !== backlinkFor($baseLink.definition)?.[0]) {
1470-
// up__ID already part of inner where exists, no need to add it explicitly here
1471-
flatKeys.push(...getFlatColumnsFor(v, { tableAlias: $baseLink.alias }))
1472-
}
1473-
}
1474-
// TODO: improve error message, the current message is generally not true (only for OData shortcut notation)
1475-
if (flatKeys.length > 1)
1476-
throw new Error('Filters can only be applied to managed associations which result in a single foreign key')
1477-
flatKeys.forEach(c => keyValComparisons.push([...[c, '=', token]]))
1478-
keyValComparisons.forEach((kv, j) =>
1479-
transformedTokenStream.push(...kv) && keyValComparisons[j + 1] ? transformedTokenStream.push('and') : null,
1480-
)
1471+
const flatKeys = getPrimaryKey(def, $baseLink.alias)
1472+
if (flatKeys.length > 1) // TODO: what about keyless?
1473+
throw new Error(`Shortcut notation “[${token.val}]” not available for composite primary key of “${def.name}”, write “<key> = ${token.val}” explicitly`)
1474+
transformedTokenStream.push(...[flatKeys[0], '=', token]);
14811475
} else if (token.ref && token.param) {
14821476
transformedTokenStream.push({ ...token })
14831477
} else if (pseudos.elements[token.ref?.[0]]) {
@@ -1785,9 +1779,10 @@ function cqn4sql(originalQuery, model) {
17851779
}
17861780
}
17871781

1788-
// only append infix filter to outer where if it is the leaf of the from ref
1789-
if (refReverse[0].where)
1782+
// OData variant w/o mentioning key
1783+
if (refReverse[0].where?.length === 1 && refReverse[0].where[0].val) {
17901784
filterConditions.push(getTransformedTokenStream(refReverse[0].where,{ $baseLink: $refLinksReverse[0] }))
1785+
}
17911786

17921787
if (existingWhere.length > 0) filterConditions.push(existingWhere)
17931788
if (whereExistsSubSelects.length > 0) {
@@ -2418,6 +2413,12 @@ function assignQueryModifiers(SELECT, modifiers) {
24182413
} else if (key === 'having') {
24192414
if (!SELECT.having) SELECT.having = val
24202415
else SELECT.having.push('and', ...val)
2416+
} else if (key === 'where') {
2417+
// ignore OData shortcut variant: `… bookshop.Orders:items[2]`
2418+
if(!val || val.length === 1 && val[0].val) continue
2419+
if (!SELECT.where) SELECT.where = val
2420+
// infix filter comes first in resulting where
2421+
else SELECT.where = [...(hasLogicalOr(val) ? [asXpr(val)] : val), 'and', ...(hasLogicalOr(SELECT.where) ? [asXpr(SELECT.where)] : SELECT.where)]
24212422
}
24222423
}
24232424
}

‎db-service/test/cqn4sql/basic.test.js‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,24 +71,20 @@ describe('query clauses', () => {
7171
Bar.ID as ID: cds.String
7272
}`)
7373
})
74-
//(SMW) TODO I'd prefer to have the cond from the filter before the cond coming from the WHERE
75-
// which, by the way, is the case in tests below where we have a path in FROM -> ???
7674
it('handles infix filter at entity and WHERE clause', () => {
77-
let query = cqn4sql(cds.ql`SELECT from bookshop.Books[price < 12.13] as Books {Books.ID} where stock < 11`, model)
75+
let query = cqn4sql(cds.ql`SELECT from bookshop.Books[price < 12.13 or true] as Books {Books.ID} where stock < 11`, model)
7876
expect(query).to.deep.equal(
79-
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.stock < 11) and (Books.price < 12.13)`,
77+
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.price < 12.13 or true) and Books.stock < 11`,
8078
)
8179
})
8280

83-
//(SMW) TODO I'd prefer to have the cond from the filter before the cond coming from the WHERE
84-
// which, by the way, is the case in tests below where we have a path in FROM -> ???
8581
it('gets precedence right for infix filter at entity and WHERE clause', () => {
8682
let query = cqn4sql(
8783
cds.ql`SELECT from bookshop.Books[price < 12.13 or stock > 77] as Books {Books.ID} where stock < 11 or price > 17.89`,
8884
model,
8985
)
9086
expect(query).to.deep.equal(
91-
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.stock < 11 or Books.price > 17.89) and (Books.price < 12.13 or Books.stock > 77)`,
87+
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.price < 12.13 or Books.stock > 77) and (Books.stock < 11 or Books.price > 17.89)`,
9288
)
9389
//expect (query) .to.deep.equal (cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.price < 12.13 or Books.stock > 77) and (Books.stock < 11 or Books.price > 17.89)`) // (SMW) want this
9490
})
@@ -101,7 +97,7 @@ describe('query clauses', () => {
10197
model,
10298
)
10399
expect(query).to.deep.equal(
104-
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.stock < 11) and (not (Books.price < 12.13))`,
100+
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE not (Books.price < 12.13) and Books.stock < 11`,
105101
)
106102
})
107103

‎db-service/test/cqn4sql/exists.test/negative.spec.js‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ describe('(exist predicate) negative tests', () => {
8585
})
8686

8787
describe('restrictions', () => {
88-
it('rejects the path expression at the leaf of scoped queries', () => {
88+
// semantically equivalent to adding a where clause..
89+
// IMO artificially rejecting this is not necessary, we can solve this uniformly also for regular where clause
90+
it.skip('rejects the path expression at the leaf of scoped queries', () => {
8991
// original idea was to just add the `genre.name` as WHERE clause to the query
9092
// however, with left outer joins we might get too many results
9193
//
@@ -104,12 +106,9 @@ describe('(exist predicate) negative tests', () => {
104106
)
105107
})
106108

107-
// (SMW) msg not good -> filter in general is ok for assoc with multiple FKS,
108-
// only shortcut notation is not allowed
109-
// TODO: message is BAD, it could include the fix: `write ”<key> = 42” explicitly`
110109
it('OData shortcut notation does not work on associations with multiple foreign keys', () => {
111110
expect(() => cqn4sql(cds.ql`SELECT from bookshop.AssocWithStructuredKey:toStructuredKey[42]`)).to.throw(
112-
/Filters can only be applied to managed associations which result in a single foreign key/,
111+
`Shortcut notation “[42]” not available for composite primary key of “bookshop.WithStructuredKey”, write “<key> = 42” explicitly`,
113112
)
114113
})
115114
})

‎db-service/test/cqn4sql/exists.test/scoped-queries.spec.js‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ describe('(exist predicate) scoped queries', () => {
322322
WHERE EXISTS (
323323
SELECT 1 from bookshop.Books as $B
324324
where $B.author_ID = author.ID and ($B.ID=201 or $B.ID=202)
325-
) and (author.ID=4711 or author.ID=4712) and (author.name='foo' or author.name='bar')`
325+
) and ((author.ID=4711 or author.ID=4712) and (author.name='foo' or author.name='bar'))`
326326

327327
expectCqn(transformed).to.equal(expected)
328328
})
@@ -450,11 +450,11 @@ describe('(exist predicate) scoped queries', () => {
450450
$a.ID
451451
}
452452
WHERE EXISTS (
453-
SELECT 1 from bookshop.Books as $B
454-
where $B.author_ID = $a.ID
453+
SELECT 1 from bookshop.Books as $B2
454+
where $B2.author_ID = $a.ID
455455
) and EXISTS (
456-
SELECT 1 from bookshop.Books as $b2
457-
where $b2.author_ID = $a.ID
456+
SELECT 1 from bookshop.Books as $b
457+
where $b.author_ID = $a.ID
458458
)`
459459

460460
expectCqn(transformed).to.equal(expected)
@@ -497,15 +497,15 @@ describe('(exist predicate) scoped queries', () => {
497497
$a.ID
498498
}
499499
WHERE EXISTS (
500-
SELECT 1 from bookshop.Books as $B
501-
where $B.author_ID = $a.ID
500+
SELECT 1 from bookshop.Books as $B2
501+
where $B2.author_ID = $a.ID
502502
and EXISTS (
503503
SELECT 1 from bookshop.Genres as $g
504-
where $g.ID = $B.genre_ID
504+
where $g.ID = $B2.genre_ID
505505
)
506506
) and EXISTS (
507-
SELECT 1 from bookshop.Books as $b2
508-
where $b2.author_ID = $a.ID
507+
SELECT 1 from bookshop.Books as $b
508+
where $b.author_ID = $a.ID
509509
)`
510510

511511
expectCqn(transformed).to.equal(expected)
@@ -533,11 +533,11 @@ describe('(exist predicate) scoped queries', () => {
533533
and EXISTS (
534534
SELECT 1 from bookshop.Books as $B3 where $B3.author_ID = $a.ID
535535
and EXISTS (
536-
SELECT 1 from bookshop.Genres as $g where $g.ID = $B3.genre_ID
536+
SELECT 1 from bookshop.Genres as $g2 where $g2.ID = $B3.genre_ID
537537
)
538538
)
539539
) and EXISTS (
540-
SELECT 1 from bookshop.Genres as $g2 where $g2.ID = $b.genre_ID
540+
SELECT 1 from bookshop.Genres as $g where $g.ID = $b.genre_ID
541541
)`
542542

543543
expectCqn(transformed).to.equal(expected)
@@ -556,14 +556,14 @@ describe('(exist predicate) scoped queries', () => {
556556
$a.ID
557557
}
558558
WHERE EXISTS (
559-
SELECT 1 from bookshop.Books as $B where $B.author_ID = $a.ID
559+
SELECT 1 from bookshop.Books as $B2 where $B2.author_ID = $a.ID
560560
) and EXISTS (
561-
SELECT 1 from bookshop.Books as $b2 where $b2.author_ID = $a.ID
561+
SELECT 1 from bookshop.Books as $b where $b.author_ID = $a.ID
562562
and
563563
(
564564
EXISTS (
565-
SELECT 1 from bookshop.Authors as $c where $c.ID = $b2.coAuthor_ID_unmanaged
566-
) or $b2.title = 'Sturmhöhe'
565+
SELECT 1 from bookshop.Authors as $c where $c.ID = $b.coAuthor_ID_unmanaged
566+
) or $b.title = 'Sturmhöhe'
567567
)
568568
)`
569569

‎db-service/test/cqn4sql/path-in-from.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ describe('infix filter on entities', () => {
6565
] as Books {ID} where price > 5 group by price having price order by price limit 5`, model)
6666
expect(query).to.deep.equal(
6767
cds.ql`SELECT from bookshop.Books as Books {Books.ID}
68-
WHERE (Books.price > 5) and (Books.price < 12.13)
68+
WHERE Books.price < 12.13 and Books.price > 5
6969
GROUP BY Books.price, Books.title
7070
HAVING Books.price and Books.title
7171
ORDER BY Books.price, Books.title DESC

‎db-service/test/cqn4sql/search.test.js‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,21 +68,17 @@ describe('Replace attribute search by search predicate', () => {
6868
from: { as: 'Genres', ref: ['bookshop.Genres'] },
6969
where: [
7070
{
71-
xpr: [
72-
{
73-
args: [
74-
{
75-
list: [{ ref: ['Genres', 'name'] }, { ref: ['Genres', 'descr'] }, { ref: ['Genres', 'code'] }],
76-
},
77-
{ xpr: [{ val: 'x' }, 'or', { val: 'y' }] },
78-
],
79-
func: 'search',
80-
},
81-
],
71+
xpr: [{ ref: ['Genres', 'ID'] }, '<', { val: 4 }, 'or', { ref: ['Genres', 'ID'] }, '>', { val: 5 }],
8272
},
8373
'and',
8474
{
85-
xpr: [{ ref: ['Genres', 'ID'] }, '<', { val: 4 }, 'or', { ref: ['Genres', 'ID'] }, '>', { val: 5 }],
75+
args: [
76+
{
77+
list: [{ ref: ['Genres', 'name'] }, { ref: ['Genres', 'descr'] }, { ref: ['Genres', 'code'] }],
78+
},
79+
{ xpr: [{ val: 'x' }, 'or', { val: 'y' }] },
80+
],
81+
func: 'search',
8682
},
8783
],
8884
},

‎db-service/test/cqn4sql/with-parameters.test.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,18 @@ describe('entities and views with parameters', () => {
158158
expected.SELECT.where[1].SELECT.from.ref[0].args = {}
159159
expect(cqn4sql(query, model)).to.deep.equal(expected)
160160
})
161+
it('scoped query with undefined where', () => {
162+
const query = cds.ql`SELECT from Books:author(P1: 1, P2: 2) as author { ID }`
163+
// there are cases, e.g. in the odata-v2 tests, where the runtime sends a query like this
164+
query.SELECT.from.ref.at(-1).where = undefined
165+
const expected = cds.ql`
166+
SELECT from Authors(P1: 1, P2: 2) as author { author.ID }
167+
where exists (
168+
SELECT 1 from Books as $B where $B.author_ID = author.ID
169+
)
170+
`
171+
expect(cqn4sql(query, model)).to.deep.equal(expected)
172+
})
161173
})
162174

163175
describe('expand subqueries', () => {

0 commit comments

Comments
 (0)