diff --git a/db-service/lib/cqn4sql.js b/db-service/lib/cqn4sql.js index 7ecd14e21..c01306624 100644 --- a/db-service/lib/cqn4sql.js +++ b/db-service/lib/cqn4sql.js @@ -74,6 +74,12 @@ function cqn4sql(originalQuery, model) { if (inferred.SELECT?.from.ref?.at(-1).id) { assignQueryModifiers(inferred.SELECT, inferred.SELECT.from.ref.at(-1)) } + if (inferred.DELETE?.from.ref?.at(-1).id) { + assignQueryModifiers(inferred.DELETE, inferred.DELETE.from.ref.at(-1)) + } + if (inferred.UPDATE?.entity.ref?.at(-1).id) { + assignQueryModifiers(inferred.UPDATE, inferred.UPDATE.entity.ref.at(-1)) + } inferred = infer(inferred, model) const { getLocalizedName, isLocalized, getDefinition } = getModelUtils(model, originalQuery) // TODO: pass model to getModelUtils // if the query has custom joins we don't want to transform it @@ -1460,24 +1466,12 @@ function cqn4sql(originalQuery, model) { else transformedTokenStream.push({ list: getTransformedTokenStream(list, { $baseLink, prop: 'list' }) }) } } else if (tokenStream.length === 1 && token.val && $baseLink) { - // infix filter - OData variant w/o mentioning key --> flatten out and compare each leaf to token.val + // infix filter - OData variant w/o mentioning key const def = getDefinition($baseLink.definition.target) || $baseLink.definition - const keys = def.keys // use key aspect on entity - const keyValComparisons = [] - const flatKeys = [] - for (const v of Object.values(keys)) { - if (v !== backlinkFor($baseLink.definition)?.[0]) { - // up__ID already part of inner where exists, no need to add it explicitly here - flatKeys.push(...getFlatColumnsFor(v, { tableAlias: $baseLink.alias })) - } - } - // TODO: improve error message, the current message is generally not true (only for OData shortcut notation) - if (flatKeys.length > 1) - throw new Error('Filters can only be applied to managed associations which result in a single foreign key') - flatKeys.forEach(c => keyValComparisons.push([...[c, '=', token]])) - keyValComparisons.forEach((kv, j) => - transformedTokenStream.push(...kv) && keyValComparisons[j + 1] ? transformedTokenStream.push('and') : null, - ) + const flatKeys = getPrimaryKey(def, $baseLink.alias) + if (flatKeys.length > 1) // TODO: what about keyless? + throw new Error(`Shortcut notation “[${token.val}]” not available for composite primary key of “${def.name}”, write “ = ${token.val}” explicitly`) + transformedTokenStream.push(...[flatKeys[0], '=', token]); } else if (token.ref && token.param) { transformedTokenStream.push({ ...token }) } else if (pseudos.elements[token.ref?.[0]]) { @@ -1785,9 +1779,10 @@ function cqn4sql(originalQuery, model) { } } - // only append infix filter to outer where if it is the leaf of the from ref - if (refReverse[0].where) + // OData variant w/o mentioning key + if (refReverse[0].where?.length === 1 && refReverse[0].where[0].val) { filterConditions.push(getTransformedTokenStream(refReverse[0].where,{ $baseLink: $refLinksReverse[0] })) + } if (existingWhere.length > 0) filterConditions.push(existingWhere) if (whereExistsSubSelects.length > 0) { @@ -2418,6 +2413,12 @@ function assignQueryModifiers(SELECT, modifiers) { } else if (key === 'having') { if (!SELECT.having) SELECT.having = val else SELECT.having.push('and', ...val) + } else if (key === 'where') { + // ignore OData shortcut variant: `… bookshop.Orders:items[2]` + if(!val || val.length === 1 && val[0].val) continue + if (!SELECT.where) SELECT.where = val + // infix filter comes first in resulting where + else SELECT.where = [...(hasLogicalOr(val) ? [asXpr(val)] : val), 'and', ...(hasLogicalOr(SELECT.where) ? [asXpr(SELECT.where)] : SELECT.where)] } } } diff --git a/db-service/test/cqn4sql/basic.test.js b/db-service/test/cqn4sql/basic.test.js index d9680c945..17e5336db 100644 --- a/db-service/test/cqn4sql/basic.test.js +++ b/db-service/test/cqn4sql/basic.test.js @@ -71,24 +71,20 @@ describe('query clauses', () => { Bar.ID as ID: cds.String }`) }) - //(SMW) TODO I'd prefer to have the cond from the filter before the cond coming from the WHERE - // which, by the way, is the case in tests below where we have a path in FROM -> ??? it('handles infix filter at entity and WHERE clause', () => { - let query = cqn4sql(cds.ql`SELECT from bookshop.Books[price < 12.13] as Books {Books.ID} where stock < 11`, model) + let query = cqn4sql(cds.ql`SELECT from bookshop.Books[price < 12.13 or true] as Books {Books.ID} where stock < 11`, model) expect(query).to.deep.equal( - cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.stock < 11) and (Books.price < 12.13)`, + cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.price < 12.13 or true) and Books.stock < 11`, ) }) - //(SMW) TODO I'd prefer to have the cond from the filter before the cond coming from the WHERE - // which, by the way, is the case in tests below where we have a path in FROM -> ??? it('gets precedence right for infix filter at entity and WHERE clause', () => { let query = cqn4sql( cds.ql`SELECT from bookshop.Books[price < 12.13 or stock > 77] as Books {Books.ID} where stock < 11 or price > 17.89`, model, ) expect(query).to.deep.equal( - 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)`, + 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)`, ) //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 }) @@ -101,7 +97,7 @@ describe('query clauses', () => { model, ) expect(query).to.deep.equal( - cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.stock < 11) and (not (Books.price < 12.13))`, + cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE not (Books.price < 12.13) and Books.stock < 11`, ) }) diff --git a/db-service/test/cqn4sql/exists.test/negative.spec.js b/db-service/test/cqn4sql/exists.test/negative.spec.js index ac4e1f55c..f292188a6 100644 --- a/db-service/test/cqn4sql/exists.test/negative.spec.js +++ b/db-service/test/cqn4sql/exists.test/negative.spec.js @@ -85,7 +85,9 @@ describe('(exist predicate) negative tests', () => { }) describe('restrictions', () => { - it('rejects the path expression at the leaf of scoped queries', () => { + // semantically equivalent to adding a where clause.. + // IMO artificially rejecting this is not necessary, we can solve this uniformly also for regular where clause + it.skip('rejects the path expression at the leaf of scoped queries', () => { // original idea was to just add the `genre.name` as WHERE clause to the query // however, with left outer joins we might get too many results // @@ -104,12 +106,9 @@ describe('(exist predicate) negative tests', () => { ) }) - // (SMW) msg not good -> filter in general is ok for assoc with multiple FKS, - // only shortcut notation is not allowed - // TODO: message is BAD, it could include the fix: `write ” = 42” explicitly` it('OData shortcut notation does not work on associations with multiple foreign keys', () => { expect(() => cqn4sql(cds.ql`SELECT from bookshop.AssocWithStructuredKey:toStructuredKey[42]`)).to.throw( - /Filters can only be applied to managed associations which result in a single foreign key/, + `Shortcut notation “[42]” not available for composite primary key of “bookshop.WithStructuredKey”, write “ = 42” explicitly`, ) }) }) diff --git a/db-service/test/cqn4sql/exists.test/scoped-queries.spec.js b/db-service/test/cqn4sql/exists.test/scoped-queries.spec.js index fe81bb77c..01fa50676 100644 --- a/db-service/test/cqn4sql/exists.test/scoped-queries.spec.js +++ b/db-service/test/cqn4sql/exists.test/scoped-queries.spec.js @@ -322,7 +322,7 @@ describe('(exist predicate) scoped queries', () => { WHERE EXISTS ( SELECT 1 from bookshop.Books as $B where $B.author_ID = author.ID and ($B.ID=201 or $B.ID=202) - ) and (author.ID=4711 or author.ID=4712) and (author.name='foo' or author.name='bar')` + ) and ((author.ID=4711 or author.ID=4712) and (author.name='foo' or author.name='bar'))` expectCqn(transformed).to.equal(expected) }) @@ -450,11 +450,11 @@ describe('(exist predicate) scoped queries', () => { $a.ID } WHERE EXISTS ( - SELECT 1 from bookshop.Books as $B - where $B.author_ID = $a.ID + SELECT 1 from bookshop.Books as $B2 + where $B2.author_ID = $a.ID ) and EXISTS ( - SELECT 1 from bookshop.Books as $b2 - where $b2.author_ID = $a.ID + SELECT 1 from bookshop.Books as $b + where $b.author_ID = $a.ID )` expectCqn(transformed).to.equal(expected) @@ -497,15 +497,15 @@ describe('(exist predicate) scoped queries', () => { $a.ID } WHERE EXISTS ( - SELECT 1 from bookshop.Books as $B - where $B.author_ID = $a.ID + SELECT 1 from bookshop.Books as $B2 + where $B2.author_ID = $a.ID and EXISTS ( SELECT 1 from bookshop.Genres as $g - where $g.ID = $B.genre_ID + where $g.ID = $B2.genre_ID ) ) and EXISTS ( - SELECT 1 from bookshop.Books as $b2 - where $b2.author_ID = $a.ID + SELECT 1 from bookshop.Books as $b + where $b.author_ID = $a.ID )` expectCqn(transformed).to.equal(expected) @@ -533,11 +533,11 @@ describe('(exist predicate) scoped queries', () => { and EXISTS ( SELECT 1 from bookshop.Books as $B3 where $B3.author_ID = $a.ID and EXISTS ( - SELECT 1 from bookshop.Genres as $g where $g.ID = $B3.genre_ID + SELECT 1 from bookshop.Genres as $g2 where $g2.ID = $B3.genre_ID ) ) ) and EXISTS ( - SELECT 1 from bookshop.Genres as $g2 where $g2.ID = $b.genre_ID + SELECT 1 from bookshop.Genres as $g where $g.ID = $b.genre_ID )` expectCqn(transformed).to.equal(expected) @@ -556,14 +556,14 @@ describe('(exist predicate) scoped queries', () => { $a.ID } WHERE EXISTS ( - SELECT 1 from bookshop.Books as $B where $B.author_ID = $a.ID + SELECT 1 from bookshop.Books as $B2 where $B2.author_ID = $a.ID ) and EXISTS ( - SELECT 1 from bookshop.Books as $b2 where $b2.author_ID = $a.ID + SELECT 1 from bookshop.Books as $b where $b.author_ID = $a.ID and ( EXISTS ( - SELECT 1 from bookshop.Authors as $c where $c.ID = $b2.coAuthor_ID_unmanaged - ) or $b2.title = 'Sturmhöhe' + SELECT 1 from bookshop.Authors as $c where $c.ID = $b.coAuthor_ID_unmanaged + ) or $b.title = 'Sturmhöhe' ) )` diff --git a/db-service/test/cqn4sql/path-in-from.test.js b/db-service/test/cqn4sql/path-in-from.test.js index 6d475e9b2..62cce34db 100644 --- a/db-service/test/cqn4sql/path-in-from.test.js +++ b/db-service/test/cqn4sql/path-in-from.test.js @@ -65,7 +65,7 @@ describe('infix filter on entities', () => { ] as Books {ID} where price > 5 group by price having price order by price limit 5`, model) expect(query).to.deep.equal( cds.ql`SELECT from bookshop.Books as Books {Books.ID} - WHERE (Books.price > 5) and (Books.price < 12.13) + WHERE Books.price < 12.13 and Books.price > 5 GROUP BY Books.price, Books.title HAVING Books.price and Books.title ORDER BY Books.price, Books.title DESC diff --git a/db-service/test/cqn4sql/search.test.js b/db-service/test/cqn4sql/search.test.js index 067f74a96..7905dc9f4 100644 --- a/db-service/test/cqn4sql/search.test.js +++ b/db-service/test/cqn4sql/search.test.js @@ -68,21 +68,17 @@ describe('Replace attribute search by search predicate', () => { from: { as: 'Genres', ref: ['bookshop.Genres'] }, where: [ { - xpr: [ - { - args: [ - { - list: [{ ref: ['Genres', 'name'] }, { ref: ['Genres', 'descr'] }, { ref: ['Genres', 'code'] }], - }, - { xpr: [{ val: 'x' }, 'or', { val: 'y' }] }, - ], - func: 'search', - }, - ], + xpr: [{ ref: ['Genres', 'ID'] }, '<', { val: 4 }, 'or', { ref: ['Genres', 'ID'] }, '>', { val: 5 }], }, 'and', { - xpr: [{ ref: ['Genres', 'ID'] }, '<', { val: 4 }, 'or', { ref: ['Genres', 'ID'] }, '>', { val: 5 }], + args: [ + { + list: [{ ref: ['Genres', 'name'] }, { ref: ['Genres', 'descr'] }, { ref: ['Genres', 'code'] }], + }, + { xpr: [{ val: 'x' }, 'or', { val: 'y' }] }, + ], + func: 'search', }, ], }, diff --git a/db-service/test/cqn4sql/with-parameters.test.js b/db-service/test/cqn4sql/with-parameters.test.js index 0db5300aa..2dbcbf9db 100644 --- a/db-service/test/cqn4sql/with-parameters.test.js +++ b/db-service/test/cqn4sql/with-parameters.test.js @@ -158,6 +158,18 @@ describe('entities and views with parameters', () => { expected.SELECT.where[1].SELECT.from.ref[0].args = {} expect(cqn4sql(query, model)).to.deep.equal(expected) }) + it('scoped query with undefined where', () => { + const query = cds.ql`SELECT from Books:author(P1: 1, P2: 2) as author { ID }` + // there are cases, e.g. in the odata-v2 tests, where the runtime sends a query like this + query.SELECT.from.ref.at(-1).where = undefined + const expected = cds.ql` + SELECT from Authors(P1: 1, P2: 2) as author { author.ID } + where exists ( + SELECT 1 from Books as $B where $B.author_ID = author.ID + ) + ` + expect(cqn4sql(query, model)).to.deep.equal(expected) + }) }) describe('expand subqueries', () => {