From 4ca6c662e9d9ab2418f834f10a2595b84111c47b Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Mon, 8 Jan 2024 13:49:31 +0100 Subject: [PATCH 1/8] Add fallback for unknown entity hana --- hana/lib/HANAService.js | 20 ++++++++++++++++++++ test/scenarios/bookshop/update.test.js | 19 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index 2ee1438a5..217017331 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -532,6 +532,14 @@ class HANAService extends SQLService { : ObjectKeys(INSERT.entries[0]) this.columns = columns.filter(elements ? c => !elements[c]?.['@cds.extension'] : () => true) + // Fallback to plain insert + if (!elements) { + this.entries = INSERT.entries.map(e => columns.map(c => e[c])) + return `INSERT INTO ${this.quote(entity)} (${columns.map(c => + this.quote(c), + )}) VALUES (${columns.map(() => '?')})` + } + const extractions = this.managed( columns.map(c => ({ name: c })), elements, @@ -599,6 +607,18 @@ class HANAService extends SQLService { // Recommendation is to always use entries const elements = q.elements || q.target?.elements const columns = INSERT.columns || (elements && ObjectKeys(elements)) + + // Fallback to plain insert + if (!elements) { + // REVISIT: should @cds.persistence.name be considered ? + const entity = q.target?.['@cds.persistence.name'] || this.name(q.target?.name || INSERT.into.ref[0]) + + this.entries = INSERT.rows + return `INSERT INTO ${this.quote(entity)} (${columns.map(c => + this.quote(c), + )}) VALUES (${columns.map(() => '?')})` + } + const entries = new Array(INSERT.rows.length) const rows = INSERT.rows for (let x = 0; x < rows.length; x++) { diff --git a/test/scenarios/bookshop/update.test.js b/test/scenarios/bookshop/update.test.js index 28fdcd9ff..8ffd1d0a7 100644 --- a/test/scenarios/bookshop/update.test.js +++ b/test/scenarios/bookshop/update.test.js @@ -24,6 +24,7 @@ describe('Bookshop - Update', () => { expect(res.data.author_ID).to.be.eq(201) expect(res.data.descr).to.be.eq('UPDATED') }) + test('Update array of', async () => { // create book const insert = INSERT.into('sap.capire.bookshop.Books').columns(['ID']).values([150]) @@ -41,6 +42,24 @@ describe('Bookshop - Update', () => { expect(update.data.footnotes).to.be.eql(['one']) }) + test('programmatic insert into unknown entity', async () => { + const books = 'sap_capire_bookshop_Books' + const id = 999 + let affectedRows = await INSERT(books, { + ID, + modifiedAt: new Date(), + createdAt: (new Date()).toISOString(), + }) + expect(affectedRows).to.be.eq(1) + + await DELETE(books).where(`ID = `, { val: ID }) + + affectedRows = await INSERT.into(books) + .columns(['ID', 'modifiedAt', 'createdAt']) + .values([ID, new Date(), (new Date()).toISOString()]) + expect(affectedRows).to.be.eq(1) + }) + test('programmatic update without body incl. managed', async () => { const { modifiedAt } = await cds.db.run(cds.ql.SELECT.from('sap.capire.bookshop.Books', { ID: 251 })) const affectedRows = await cds.db.run(cds.ql.UPDATE('sap.capire.bookshop.Books', { ID: 251 })) From 63173a7a2c063a6d2b5a007485ba31e500d3ca09 Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Mon, 8 Jan 2024 13:54:03 +0100 Subject: [PATCH 2/8] linting --- hana/lib/HANAService.js | 2 +- hana/lib/drivers/hdb.js | 2 +- test/scenarios/bookshop/update.test.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index 217017331..3684b0be5 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -275,7 +275,7 @@ class HANAService extends SQLService { q.as = alias const src = q - const { limit, one, orderBy, expand, columns, localized, count, from, parent } = q.SELECT + const { limit, one, orderBy, expand, columns, localized, count, parent } = q.SELECT // When one of these is defined wrap the query in a sub query if (expand || (parent && (limit || one || orderBy))) { diff --git a/hana/lib/drivers/hdb.js b/hana/lib/drivers/hdb.js index e5fba40b0..bc30bcb4e 100644 --- a/hana/lib/drivers/hdb.js +++ b/hana/lib/drivers/hdb.js @@ -145,7 +145,7 @@ async function* rsIterator(rs, one) { this.reading = 0 this.writing = 0 }) - .catch(e => { + .catch(() => { // TODO: check whether the error is early close return true }) diff --git a/test/scenarios/bookshop/update.test.js b/test/scenarios/bookshop/update.test.js index 8ffd1d0a7..1899e1857 100644 --- a/test/scenarios/bookshop/update.test.js +++ b/test/scenarios/bookshop/update.test.js @@ -44,7 +44,7 @@ describe('Bookshop - Update', () => { test('programmatic insert into unknown entity', async () => { const books = 'sap_capire_bookshop_Books' - const id = 999 + const ID = 999 let affectedRows = await INSERT(books, { ID, modifiedAt: new Date(), From cf9ab47735d5cdad65082fd642691c213cf4ddf9 Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Mon, 8 Jan 2024 14:04:47 +0100 Subject: [PATCH 3/8] Adjust unkown entity test --- test/scenarios/bookshop/update.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/scenarios/bookshop/update.test.js b/test/scenarios/bookshop/update.test.js index 1899e1857..89e1a9419 100644 --- a/test/scenarios/bookshop/update.test.js +++ b/test/scenarios/bookshop/update.test.js @@ -45,7 +45,8 @@ describe('Bookshop - Update', () => { test('programmatic insert into unknown entity', async () => { const books = 'sap_capire_bookshop_Books' const ID = 999 - let affectedRows = await INSERT(books, { + let affectedRows = await INSERT.into(books) + .entries({ ID, modifiedAt: new Date(), createdAt: (new Date()).toISOString(), From b615201ddcba049f06cafea8092683119969e34f Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Tue, 9 Jan 2024 10:44:13 +0100 Subject: [PATCH 4/8] Move fallback insert behavior to cqn2sql --- db-service/lib/cqn2sql.js | 13 +++++++++++++ hana/lib/HANAService.js | 27 ++++++-------------------- test/scenarios/bookshop/update.test.js | 23 +++++++++++----------- 3 files changed, 30 insertions(+), 33 deletions(-) diff --git a/db-service/lib/cqn2sql.js b/db-service/lib/cqn2sql.js index 4457c544d..6572802ad 100644 --- a/db-service/lib/cqn2sql.js +++ b/db-service/lib/cqn2sql.js @@ -400,6 +400,12 @@ class CQN2SQLRenderer { /** @type {string[]} */ this.columns = columns.filter(elements ? c => !elements[c]?.['@cds.extension'] : () => true).map(c => this.quote(c)) + if (!elements) { + this.entries = INSERT.entries.map(e => columns.map(c => e[c])) + const param = this.param.bind(this, { ref: ['?'] }) + return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns}) VALUES (${columns.map(param)})`) + } + const extractions = this.managed( columns.map(c => ({ name: c })), elements, @@ -453,6 +459,13 @@ class CQN2SQLRenderer { }) this.columns = columns.map(c => this.quote(c)) + + if (!elements) { + this.entries = INSERT.rows + const param = this.param.bind(this, { ref: ['?'] }) + return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns}) VALUES (${columns.map(param)})`) + } + this.entries = [[JSON.stringify(INSERT.rows)]] return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns }) SELECT ${extraction} FROM json_each(?)`) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index 3684b0be5..be5403f07 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -524,22 +524,15 @@ class HANAService extends SQLService { const entity = q.target?.['@cds.persistence.name'] || this.name(q.target?.name || INSERT.into.ref[0]) const elements = q.elements || q.target?.elements - if (!elements && !INSERT.entries?.length) { - return // REVISIT: mtx sends an insert statement without entries and no reference entity + if (!elements) { + return super.INSERT_entries(q) } + const columns = elements ? ObjectKeys(elements).filter(c => c in elements && !elements[c].virtual && !elements[c].value && !elements[c].isAssociation) : ObjectKeys(INSERT.entries[0]) this.columns = columns.filter(elements ? c => !elements[c]?.['@cds.extension'] : () => true) - // Fallback to plain insert - if (!elements) { - this.entries = INSERT.entries.map(e => columns.map(c => e[c])) - return `INSERT INTO ${this.quote(entity)} (${columns.map(c => - this.quote(c), - )}) VALUES (${columns.map(() => '?')})` - } - const extractions = this.managed( columns.map(c => ({ name: c })), elements, @@ -606,19 +599,11 @@ class HANAService extends SQLService { // The problem with Simple INSERT is the type mismatch from csv files // Recommendation is to always use entries const elements = q.elements || q.target?.elements - const columns = INSERT.columns || (elements && ObjectKeys(elements)) - - // Fallback to plain insert if (!elements) { - // REVISIT: should @cds.persistence.name be considered ? - const entity = q.target?.['@cds.persistence.name'] || this.name(q.target?.name || INSERT.into.ref[0]) - - this.entries = INSERT.rows - return `INSERT INTO ${this.quote(entity)} (${columns.map(c => - this.quote(c), - )}) VALUES (${columns.map(() => '?')})` + return super.INSERT_rows(q) } - + + const columns = INSERT.columns || (elements && ObjectKeys(elements)) const entries = new Array(INSERT.rows.length) const rows = INSERT.rows for (let x = 0; x < rows.length; x++) { diff --git a/test/scenarios/bookshop/update.test.js b/test/scenarios/bookshop/update.test.js index 89e1a9419..6fc3325f5 100644 --- a/test/scenarios/bookshop/update.test.js +++ b/test/scenarios/bookshop/update.test.js @@ -47,30 +47,29 @@ describe('Bookshop - Update', () => { const ID = 999 let affectedRows = await INSERT.into(books) .entries({ - ID, - modifiedAt: new Date(), - createdAt: (new Date()).toISOString(), - }) - expect(affectedRows).to.be.eq(1) + ID, + createdAt: (new Date()).toISOString(), + }) + expect(affectedRows|0).to.be.eq(1) await DELETE(books).where(`ID = `, { val: ID }) affectedRows = await INSERT.into(books) - .columns(['ID', 'modifiedAt', 'createdAt']) - .values([ID, new Date(), (new Date()).toISOString()]) - expect(affectedRows).to.be.eq(1) + .columns(['ID', 'createdAt']) + .values([ID, (new Date()).toISOString()]) + expect(affectedRows|0).to.be.eq(1) }) test('programmatic update without body incl. managed', async () => { - const { modifiedAt } = await cds.db.run(cds.ql.SELECT.from('sap.capire.bookshop.Books', { ID: 251 })) - const affectedRows = await cds.db.run(cds.ql.UPDATE('sap.capire.bookshop.Books', { ID: 251 })) + const { modifiedAt } = await SELECT.from('sap.capire.bookshop.Books', { ID: 251 }) + const affectedRows = await UPDATE('sap.capire.bookshop.Books', { ID: 251 }) expect(affectedRows).to.be.eq(1) - const { modifiedAt: newModifiedAt } = await cds.db.run(cds.ql.SELECT.from('sap.capire.bookshop.Books', { ID: 251 })) + const { modifiedAt: newModifiedAt } = await SELECT.from('sap.capire.bookshop.Books', { ID: 251 }) expect(newModifiedAt).not.to.be.eq(modifiedAt) }) test('programmatic update without body excl. managed', async () => { - const affectedRows = await cds.db.run(cds.ql.UPDATE('sap.capire.bookshop.Genres', { ID: 10 })) + const affectedRows = await UPDATE('sap.capire.bookshop.Genres', { ID: 10 }) expect(affectedRows).to.be.eq(0) }) From 0fd9573698f5597e1396f492f2ea0d97d175f0a9 Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Wed, 10 Jan 2024 10:42:43 +0100 Subject: [PATCH 5/8] enhance unknown entity test with all operations --- db-service/lib/SQLService.js | 9 +++++- test/scenarios/bookshop/update.test.js | 39 +++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/db-service/lib/SQLService.js b/db-service/lib/SQLService.js index d9dd657cb..339d9e2ca 100644 --- a/db-service/lib/SQLService.js +++ b/db-service/lib/SQLService.js @@ -297,7 +297,14 @@ class SQLService extends DatabaseService { * @returns {import('./infer/cqn').Query} */ cqn4sql(q) { - if (!q.SELECT?.from?.join && !q.SELECT?.from?.SELECT && !this.model?.definitions[_target_name4(q)]) return _unquirked(q) + if ( + !cds.env.features.db_strict && + !q.SELECT?.from?.join && + !q.SELECT?.from?.SELECT && + !this.model?.definitions[_target_name4(q)] + ) { + return _unquirked(q) + } return cqn4sql(q, this.model) } diff --git a/test/scenarios/bookshop/update.test.js b/test/scenarios/bookshop/update.test.js index 6fc3325f5..e19414f5a 100644 --- a/test/scenarios/bookshop/update.test.js +++ b/test/scenarios/bookshop/update.test.js @@ -42,7 +42,7 @@ describe('Bookshop - Update', () => { expect(update.data.footnotes).to.be.eql(['one']) }) - test('programmatic insert into unknown entity', async () => { + test('programmatic insert/upsert/update/delete with unknown entity', async () => { const books = 'sap_capire_bookshop_Books' const ID = 999 let affectedRows = await INSERT.into(books) @@ -50,14 +50,45 @@ describe('Bookshop - Update', () => { ID, createdAt: (new Date()).toISOString(), }) - expect(affectedRows|0).to.be.eq(1) + expect(affectedRows | 0).to.be.eq(1) - await DELETE(books).where(`ID = `, { val: ID }) + affectedRows = await DELETE(books) + .where({ ID }) + expect(affectedRows | 0).to.be.eq(1) affectedRows = await INSERT.into(books) .columns(['ID', 'createdAt']) .values([ID, (new Date()).toISOString()]) - expect(affectedRows|0).to.be.eq(1) + expect(affectedRows | 0).to.be.eq(1) + + affectedRows = await UPDATE(books) + .with({ modifiedAt: (new Date()).toISOString() }) + .where({ ID }) + expect(affectedRows | 0).to.be.eq(1) + + affectedRows = await DELETE(books) + .where({ ID }) + expect(affectedRows | 0).to.be.eq(1) + + // UPSERT fallback to an INSERT + affectedRows = await UPSERT.into(books) + .entries({ + ID, + createdAt: (new Date()).toISOString(), + }) + expect(affectedRows | 0).to.be.eq(1) + + // UPSERT fallback to an INSERT (throws on secondary call) + affectedRows = UPSERT.into(books) + .entries({ + ID, + createdAt: (new Date()).toISOString(), + }) + await expect(affectedRows).rejected + + affectedRows = await DELETE(books) + .where({ ID }) + expect(affectedRows | 0).to.be.eq(1) }) test('programmatic update without body incl. managed', async () => { From 5aadd8c3a1fb362b8111648f873ce444f8dad220 Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Wed, 10 Jan 2024 13:01:28 +0100 Subject: [PATCH 6/8] return next call as is required --- db-service/lib/SQLService.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/db-service/lib/SQLService.js b/db-service/lib/SQLService.js index 339d9e2ca..edcba3095 100644 --- a/db-service/lib/SQLService.js +++ b/db-service/lib/SQLService.js @@ -45,7 +45,7 @@ class SQLService extends DatabaseService { async transformStreamIntoCQN({ query, data, target }, next) { let col, type, etag const elements = query._target?.elements || target?.elements - if (!elements) next() + if (!elements) return next() for (const key in elements) { const element = elements[key] if (element['@Core.MediaType'] && data[key]?.pipe) col = key @@ -163,22 +163,22 @@ class SQLService extends DatabaseService { if (typeof from === 'string') from = { ref: [from] } if (where) { let last = from.ref.at(-1) - if (last.where) [ last, where ] = [ last.id, [ { xpr: last.where }, 'and', { xpr: where } ] ] - from = {ref:[ ...from.ref.slice(0,-1), { id: last, where }]} + if (last.where) [last, where] = [last.id, [{ xpr: last.where }, 'and', { xpr: where }]] + from = { ref: [...from.ref.slice(0, -1), { id: last, where }] } } // Process child compositions depth-first - let { depth=0, visited=[] } = req - visited.push (req.target.name) - await Promise.all (Object.values(compositions).map(c => { + let { depth = 0, visited = [] } = req + visited.push(req.target.name) + await Promise.all(Object.values(compositions).map(c => { if (c._target['@cds.persistence.skip'] === true) return if (c._target === req.target) { // the Genre.children case if (++depth > (c['@depth'] || 3)) return } else if (visited.includes(c._target.name)) throw new Error( - `Transitive circular composition detected: \n\n`+ - ` ${visited.join(' > ')} > ${c._target.name} \n\n`+ + `Transitive circular composition detected: \n\n` + + ` ${visited.join(' > ')} > ${c._target.name} \n\n` + `These are not supported by deep delete.`) // Prepare and run deep query, à la CQL`DELETE from Foo[pred]:comp1.comp2...` - const query = DELETE.from({ref:[ ...from.ref, c.name ]}) + const query = DELETE.from({ ref: [...from.ref, c.name] }) return this.onDELETE({ query, depth, visited: [...visited], target: c._target }) })) } @@ -233,8 +233,8 @@ class SQLService extends DatabaseService { // REVISIT: made uppercase count because of HANA reserved word quoting const cq = SELECT.one([{ func: 'count', as: 'COUNT' }]).from( cds.ql.clone(query, { - localized: false, - expand: false, + localized: false, + expand: false, limit: undefined, orderBy: undefined, }), From 0152c06efc6a2a11ba93efd114894ee3d4b7d041 Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Wed, 10 Jan 2024 13:08:42 +0100 Subject: [PATCH 7/8] only calculate insert type when required --- hana/lib/HANAService.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index 2e8f7b582..57d663424 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -814,8 +814,7 @@ class HANAService extends SQLService { const val = _managed[element[annotation]?.['=']] let managed if (val) managed = this.func({ func: 'session_context', args: [{ val, param: false }] }) - const type = this.insertType4(element) - let extract = sql ?? `${this.quote(name)} ${type} PATH '$.${name}'` + let extract = sql ?? `${this.quote(name)} ${this.insertType4(element)} PATH '$.${name}'` if (!isUpdate) { const d = element.default if (d && (d.val !== undefined || d.ref?.[0] === '$now')) { From fc4b8fe1bf2b40122e8598e4491fd067038eb469 Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Thu, 11 Jan 2024 10:53:59 +0100 Subject: [PATCH 8/8] fallback UPSERT to INSERT query --- hana/lib/HANAService.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index 57d663424..46b1e279a 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -604,13 +604,17 @@ class HANAService extends SQLService { } UPSERT(q) { - let { UPSERT } = q, - sql = this.INSERT({ __proto__: q, INSERT: UPSERT }) + const { UPSERT } = q + const sql = this.INSERT({ __proto__: q, INSERT: UPSERT }) - // REVISIT: should @cds.persistence.name be considered ? - const entity = q.target?.['@cds.persistence.name'] || this.name(q.target?.name || INSERT.into.ref[0]) + // If no definition is available fallback to INSERT statement const elements = q.elements || q.target?.elements + if (!elements) { + return (this.sql = sql) + } + // REVISIT: should @cds.persistence.name be considered ? + const entity = q.target?.['@cds.persistence.name'] || this.name(q.target?.name || INSERT.into.ref[0]) const dataSelect = sql.substring(sql.indexOf('WITH')) // Calculate @cds.on.insert