chore: re-write filter at from.at(-1) into queries where

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 “<key> = ${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)]
     }
   }
 }

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`,
     )
   })
 

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 ”<key> = 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 “<key> = 42” explicitly`,
       )
     })
   })

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'
           )
         )`
 

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

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',
           },
         ],
       },

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', () => {