From 94f077661cffad8f137dc692a2cb9b0ae5e4d75b Mon Sep 17 00:00:00 2001 From: Patrice Bender Date: Tue, 31 Oct 2023 13:24:38 +0100 Subject: [PATCH] feat(`UPDATE`/`DELETE`): Enable path expressions for improved data manipulation (#325) Co-authored-by: Johannes Vogel <31311694+johannes-vogel@users.noreply.github.com> --- db-service/lib/cqn4sql.js | 28 +++++- db-service/lib/infer/index.js | 4 - db-service/test/cqn4sql/DELETE.test.js | 121 +++++++++++++++---------- db-service/test/cqn4sql/UPDATE.test.js | 53 +++++++++-- test/bookshop/srv/admin-service.cds | 7 ++ test/compliance/DELETE.test.js | 18 +++- 6 files changed, 166 insertions(+), 65 deletions(-) diff --git a/db-service/lib/cqn4sql.js b/db-service/lib/cqn4sql.js index 87d2a8d08..8fef738fd 100644 --- a/db-service/lib/cqn4sql.js +++ b/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) + } } } } diff --git a/db-service/lib/infer/index.js b/db-service/lib/infer/index.js index 6810d0320..2476750f0 100644 --- a/db-service/lib/infer/index.js +++ b/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) } diff --git a/db-service/test/cqn4sql/DELETE.test.js b/db-service/test/cqn4sql/DELETE.test.js index c78c65417..9f8b370f8 100644 --- a/db-service/test/cqn4sql/DELETE.test.js +++ b/db-service/test/cqn4sql/DELETE.test.js @@ -69,6 +69,43 @@ 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 @@ -76,73 +113,57 @@ describe('DELETE', () => { 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) }) diff --git a/db-service/test/cqn4sql/UPDATE.test.js b/db-service/test/cqn4sql/UPDATE.test.js index c3af06b51..7e23ffe44 100644 --- a/db-service/test/cqn4sql/UPDATE.test.js +++ b/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 diff --git a/test/bookshop/srv/admin-service.cds b/test/bookshop/srv/admin-service.cds index c5048e58e..fd36161f5 100644 --- a/test/bookshop/srv/admin-service.cds +++ b/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 + } } diff --git a/test/compliance/DELETE.test.js b/test/compliance/DELETE.test.js index 8c659a6f5..10a0105b2 100644 --- a/test/compliance/DELETE.test.js +++ b/test/compliance/DELETE.test.js @@ -1,4 +1,10 @@ +'use strict' + +const cds = require('../cds.js') + describe('DELETE', () => { + cds.test(__dirname + '/../bookshop') + describe('from', () => { test.skip('missing', () => { throw new Error('not supported') @@ -6,8 +12,16 @@ describe('DELETE', () => { }) 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) }) }) })