feat(UPDATE/DELETE): Enable path expressions for improved data ma…

…nipulation (#325)

Co-authored-by: Johannes Vogel <31311694+johannes-vogel@users.noreply.github.com>

db-service/lib/cqn4sql.js

@@ -96,7 +96,33 @@ function cqn4sql(originalQuery, model = cds.context?.model || cds.model) {
       }
 
       if (queryNeedsJoins) {
-        transformedQuery[kind].from = translateAssocsToJoins(transformedQuery[kind].from)
+        if (inferred.UPDATE || inferred.DELETE) {
+          const prop = inferred.UPDATE ? 'UPDATE' : 'DELETE'
+          const uniqueSubqueryAlias = getNextAvailableTableAlias(transformedFrom.as)
+          const subquery = {
+            SELECT: {
+              from: { ...transformedFrom },
+              columns: [],
+              where: [...transformedProp.where],
+            },
+          }
+          // outer query gets new alias, so that we don't have to replace
+          // references to the table alias in the subquery
+          transformedFrom.as = uniqueSubqueryAlias
+          const queryTarget = Object.values(originalQuery.sources)[0]
+          const keys = Object.values(queryTarget.elements).filter(e => e.key === true)
+          const primaryKey = { list: [] }
+          keys.forEach(k => {
+            subquery.SELECT.columns.push({ ref: [k.name] })
+            primaryKey.list.push({ ref: [uniqueSubqueryAlias, k.name] })
+          })
+          const transformedSubquery = cqn4sql(subquery)
+          transformedQuery[prop].where = [primaryKey, 'in', transformedSubquery]
+          if (prop === 'UPDATE') transformedQuery.UPDATE.entity = transformedFrom
+          else transformedQuery.DELETE.from = transformedFrom
+        } else {
+          transformedQuery[kind].from = translateAssocsToJoins(transformedQuery[kind].from)
+        }
       }
     }
   }

db-service/lib/infer/index.js

@@ -680,10 +680,6 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
           ? { ref: [...baseColumn.ref, ...column.ref], $refLinks: [...baseColumn.$refLinks, ...column.$refLinks] }
           : column
         if (isColumnJoinRelevant(colWithBase)) {
-          if (originalQuery.UPDATE)
-            throw new Error(
-              'Path expressions for UPDATE statements are not supported. Use “where exists” with infix filters instead.',
-            )
           Object.defineProperty(column, 'isJoinRelevant', { value: true })
           joinTree.mergeColumn(colWithBase, originalQuery.outerQueries)
         }

db-service/test/cqn4sql/DELETE.test.js

@@ -69,80 +69,101 @@ describe('DELETE', () => {
       }`)
     expect(query.DELETE).to.deep.equal(expected.DELETE)
   })
+  it('DELETE with where exists expansion and path expression', () => {
+    cds.model = cds.compile.for.nodejs(cds.model)
+    const { DELETE } = cds.ql
+    let d = DELETE.from('bookshop.Books:author').where(`books.title = 'Harry Potter'`)
+    const query = cqn4sql(d)
+
+    // this is the final exists subquery
+    const subquery = CQL`
+     SELECT author.ID from bookshop.Authors as author
+      left join bookshop.Books as books on books.author_ID = author.ID
+     where exists (
+      SELECT 1 from bookshop.Books as Books2 where Books2.author_ID = author.ID
+     ) and books.title = 'Harry Potter'
+    `
+    const expected = JSON.parse(`{
+      "DELETE": {
+          "from": {
+            "ref": [
+              "bookshop.Authors"
+            ],
+            "as": "author2"
+          }
+        }
+      }`)
+    expected.DELETE.where = [
+      {
+        list: [
+          {
+            ref: ['author2', 'ID'],
+          },
+        ],
+      },
+      'in',
+      subquery,
+    ]
+    expect(query.DELETE).to.deep.equal(expected.DELETE)
+  })
 
   it('DELETE with assoc filter and where exists expansion', () => {
     const { DELETE } = cds.ql
     let d = DELETE.from('bookshop.Reproduce[author = null and ID = 99]:accessGroup')
     const query = cqn4sql(d)
 
     const expected = {
-      "DELETE": {
-        "from": {
-          "ref": [
-            "bookshop.AccessGroups"
-          ],
-          "as": "accessGroup"
+      DELETE: {
+        from: {
+          ref: ['bookshop.AccessGroups'],
+          as: 'accessGroup',
         },
-        "where": [
-          "exists",
+        where: [
+          'exists',
           {
-            "SELECT": {
-              "from": {
-                "ref": [
-                  "bookshop.Reproduce"
-                ],
-                "as": "Reproduce"
+            SELECT: {
+              from: {
+                ref: ['bookshop.Reproduce'],
+                as: 'Reproduce',
               },
-              "columns": [
+              columns: [
                 {
-                  "val": 1
-                }
+                  val: 1,
+                },
               ],
-              "where": [
+              where: [
                 {
-                  "ref": [
-                    "Reproduce",
-                    "accessGroup_ID"
-                  ]
+                  ref: ['Reproduce', 'accessGroup_ID'],
                 },
-                "=",
+                '=',
                 {
-                  "ref": [
-                    "accessGroup",
-                    "ID"
-                  ]
+                  ref: ['accessGroup', 'ID'],
                 },
-                "and",
+                'and',
                 {
-                  "xpr": [
+                  xpr: [
                     {
-                      "ref": [
-                        "Reproduce",
-                        "author_ID"
-                      ]
+                      ref: ['Reproduce', 'author_ID'],
                     },
-                    "=",
+                    '=',
                     {
-                      "val": null
-                    }
-                  ]
+                      val: null,
+                    },
+                  ],
                 },
-                "and",
+                'and',
                 {
-                  "ref": [
-                    "Reproduce",
-                    "ID"
-                  ]
+                  ref: ['Reproduce', 'ID'],
                 },
-                "=",
+                '=',
                 {
-                  "val": 99
-                }
-              ]
-            }
-          }
-        ]
-      }
+                  val: 99,
+                },
+              ],
+            },
+          },
+        ],
+      },
     }
     expect(query.DELETE).to.deep.equal(expected.DELETE)
   })

db-service/test/cqn4sql/UPDATE.test.js

@@ -70,17 +70,54 @@ describe('UPDATE', () => {
     })
   })
 
-  // we do not really understand a token stream such as a where clause,
-  // hence we cannot easily rewrite a path expression into a `where exists` subquery
-  // for the moment, we should issue a proper error instead of dumping.
-  it('Update with join clause is rejected', () => {
+  it('Update with path expressions in where is handled', () => {
     const { UPDATE } = cds.ql
-    let u = UPDATE.entity('bookshop.Books').where(
+    let u = UPDATE.entity({ ref: ['bookshop.Books'] }).where(
       `author.name LIKE '%Bron%' or ( author.name LIKE '%King' and title = 'The Dark Tower') and stock >= 15`,
     )
-    expect(() => cqn4sql(u)).to.throw(
-      'Path expressions for UPDATE statements are not supported. Use “where exists” with infix filters instead.',
-    )
+
+    let expected = UPDATE.entity({ ref: ['bookshop.Books'] })
+
+    expected.UPDATE.where = [
+      { list: [{ ref: ['Books2', 'ID'] }] },
+      'in',
+      CQL`
+            (SELECT Books.ID from bookshop.Books as Books
+              left join bookshop.Authors as author on author.ID = Books.author_ID
+              where author.name LIKE '%Bron%' or ( author.name LIKE '%King' and Books.title = 'The Dark Tower') and Books.stock >= 15
+            )
+      `,
+    ]
+    expected.UPDATE.entity = {
+      as: 'Books2',
+      ref: ['bookshop.Books'],
+    }
+    let res = cqn4sql(u)
+    expect(JSON.parse(JSON.stringify(res))).to.deep.equal(JSON.parse(JSON.stringify(expected)))
+  })
+
+  it('Update with path expressions to many', () => {
+    const { UPDATE } = cds.ql
+    let u = UPDATE.entity({ ref: ['bookshop.Authors'] }).where(`books.title LIKE '%Heights%'`)
+
+    let expected = UPDATE.entity({ ref: ['bookshop.Authors'] })
+
+    expected.UPDATE.where = [
+      { list: [{ ref: ['Authors2', 'ID'] }] },
+      'in',
+      CQL`
+      (SELECT Authors.ID from bookshop.Authors as Authors
+                left join bookshop.Books as books on books.author_ID = Authors.ID
+                where books.title LIKE '%Heights%'
+              )
+    `
+    ]
+    expected.UPDATE.entity = {
+      as: 'Authors2',
+      ref: ['bookshop.Authors'],
+    }
+    let res = cqn4sql(u)
+    expect(JSON.parse(JSON.stringify(res))).to.deep.equal(JSON.parse(JSON.stringify(expected)))
   })
 
   // table alias in subquery should address Books instead of bookshop.Books

test/bookshop/srv/admin-service.cds

@@ -2,4 +2,11 @@ using { sap.capire.bookshop as my } from '../db/schema';
 service AdminService @(requires:'admin', path:'/admin') {
   entity Books as projection on my.Books;
   entity Authors as projection on my.Authors;
+
+  @cds.redirection.target: false
+  entity RenameKeys as projection on my.Books {
+    key ID as foo,
+    author,
+    author.name
+  }
 }

test/compliance/DELETE.test.js

@@ -1,13 +1,27 @@
+'use strict'
+
+const cds = require('../cds.js')
+
 describe('DELETE', () => {
+  cds.test(__dirname + '/../bookshop')
+
   describe('from', () => {
     test.skip('missing', () => {
       throw new Error('not supported')
     })
   })
 
   describe('where', () => {
-    test.skip('missing', () => {
-      throw new Error('not supported')
+    test.skip('path expressions', async () => {
+      const deleteEmilysBooks = DELETE.from('AdminService.RenameKeys').where(`author.name = 'Emily Brontë'`)
+      const selectEmilysBooks = CQL`SELECT * FROM sap.capire.bookshop.Books where author.name = 'Emily Brontë'`
+
+      const beforeDelete = await cds.run(selectEmilysBooks)
+      await cds.run(deleteEmilysBooks)
+      const afterDelete = await cds.run(selectEmilysBooks)
+
+      expect(beforeDelete).toHaveLength(1)
+      expect(afterDelete).toHaveLength(0)
     })
   })
 })