From 38575935011d26d5b0c411f7060264e9b45d1b5e Mon Sep 17 00:00:00 2001 From: Johannes Vogel Date: Mon, 18 Mar 2024 17:30:36 +0100 Subject: [PATCH 1/7] fix: joins without columns are rejected --- db-service/lib/SQLService.js | 5 +++++ test/scenarios/bookshop/read.test.js | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/db-service/lib/SQLService.js b/db-service/lib/SQLService.js index 2a86b8f66..67dea849f 100644 --- a/db-service/lib/SQLService.js +++ b/db-service/lib/SQLService.js @@ -114,6 +114,11 @@ class SQLService extends DatabaseService { * @type {Handler} */ async onSELECT({ query, data }) { + if (query.SELECT.from?.join && !query.SELECT.columns) { + throw new Error('CQN query using joins must specify the selected columns.') + } + + if (!query.target) { try { this.infer(query) } catch (e) { /**/ } } diff --git a/test/scenarios/bookshop/read.test.js b/test/scenarios/bookshop/read.test.js index 508dcffd4..b08ec4c8e 100644 --- a/test/scenarios/bookshop/read.test.js +++ b/test/scenarios/bookshop/read.test.js @@ -255,6 +255,23 @@ describe('Bookshop - Read', () => { expect((await cds.db.run(query)).count).to.be.eq(2) }) + it('joins without columns are rejected', async () => { + const query = { + SELECT: { + from: { + join: 'inner', + args: [ + { ref: ['sap.capire.bookshop.Books'], as: 'b' }, + { ref: ['sap.capire.bookshop.Authors'], as: 'a' }, + ], + on: [{ ref: ['a', 'ID'] }, '=', { ref: ['b', 'author_ID'] }], + }, + }, + } + + expect(cds.db.run(query)).to.be.rejectedWith(/joins must specify the selected columns/) + }) + test('Delete Book', async () => { const res = await DELETE('/admin/Books(271)', admin) expect(res.status).to.be.eq(204) From e415690a820c554679d3bae78063c0b9186b7ab7 Mon Sep 17 00:00:00 2001 From: Johannes Vogel Date: Mon, 18 Mar 2024 17:41:36 +0100 Subject: [PATCH 2/7] reject on hana as well --- hana/lib/HANAService.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index 5018206c1..04d078ade 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -107,6 +107,10 @@ class HANAService extends SQLService { async onSELECT(req) { const { query, data } = req + if (query.SELECT.from?.join && !query.SELECT.columns) { + throw new Error('CQN query using joins must specify the selected columns.') + } + if (!query.target) { try { this.infer(query) } catch (e) { /**/ } } From 87d5e9987e0408d4d290886a03fb4ec496eb6d07 Mon Sep 17 00:00:00 2001 From: Johannes Vogel Date: Tue, 19 Mar 2024 12:48:24 +0100 Subject: [PATCH 3/7] move check to cqn2sql --- db-service/lib/SQLService.js | 5 ----- db-service/lib/cqn2sql.js | 5 +++++ hana/lib/HANAService.js | 4 ---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/db-service/lib/SQLService.js b/db-service/lib/SQLService.js index 67dea849f..2a86b8f66 100644 --- a/db-service/lib/SQLService.js +++ b/db-service/lib/SQLService.js @@ -114,11 +114,6 @@ class SQLService extends DatabaseService { * @type {Handler} */ async onSELECT({ query, data }) { - if (query.SELECT.from?.join && !query.SELECT.columns) { - throw new Error('CQN query using joins must specify the selected columns.') - } - - if (!query.target) { try { this.infer(query) } catch (e) { /**/ } } diff --git a/db-service/lib/cqn2sql.js b/db-service/lib/cqn2sql.js index c1d035878..c128d1fce 100644 --- a/db-service/lib/cqn2sql.js +++ b/db-service/lib/cqn2sql.js @@ -207,6 +207,11 @@ class CQN2SQLRenderer { SELECT(q) { let { from, expand, where, groupBy, having, orderBy, limit, one, distinct, localized, forUpdate, forShareLock } = q.SELECT + + if (from?.join && !q.SELECT.columns) { + throw new Error('CQN query using joins must specify the selected columns.') + } + // REVISIT: When selecting from an entity that is not in the model the from.where are not normalized (as cqn4sql is skipped) if (!where && from?.ref?.length === 1 && from.ref[0]?.where) where = from.ref[0]?.where let columns = this.SELECT_columns(q) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index 04d078ade..5018206c1 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -107,10 +107,6 @@ class HANAService extends SQLService { async onSELECT(req) { const { query, data } = req - if (query.SELECT.from?.join && !query.SELECT.columns) { - throw new Error('CQN query using joins must specify the selected columns.') - } - if (!query.target) { try { this.infer(query) } catch (e) { /**/ } } From 32eb15b71b7bbc86f0da479907c8f59f97c0d82f Mon Sep 17 00:00:00 2001 From: Johannes Vogel Date: Tue, 19 Mar 2024 12:48:33 +0100 Subject: [PATCH 4/7] add test without conflicts in join --- test/scenarios/bookshop/read.test.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/test/scenarios/bookshop/read.test.js b/test/scenarios/bookshop/read.test.js index b08ec4c8e..6ab3a0f6b 100644 --- a/test/scenarios/bookshop/read.test.js +++ b/test/scenarios/bookshop/read.test.js @@ -255,7 +255,7 @@ describe('Bookshop - Read', () => { expect((await cds.db.run(query)).count).to.be.eq(2) }) - it('joins without columns are rejected', async () => { + it('joins without columns are rejected because of conflicts', async () => { const query = { SELECT: { from: { @@ -269,6 +269,23 @@ describe('Bookshop - Read', () => { }, } + expect(cds.db.run(query)).to.be.rejectedWith(/Ambiguous wildcard elements/) + }) + + it('joins without columns are rejected in general', async () => { + const query = { + SELECT: { + from: { + join: 'inner', + args: [ + { ref: ['AdminService.RenameKeys'], as: 'rk' }, + { ref: ['DraftService.DraftEnabledBooks'], as: 'deb' }, + ], + on: [{ ref: ['deb', 'ID'] }, '=', { ref: ['rk', 'foo'] }], + }, + }, + } + expect(cds.db.run(query)).to.be.rejectedWith(/joins must specify the selected columns/) }) From 70422ae0ef5dd6ebcb5f27d38fc271e84dc8a8f4 Mon Sep 17 00:00:00 2001 From: Johannes Vogel Date: Tue, 19 Mar 2024 13:23:55 +0100 Subject: [PATCH 5/7] also reject on hana --- hana/lib/HANAService.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index 5018206c1..798968ccd 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -291,6 +291,10 @@ class HANAService extends SQLService { this.temporary = this.temporary || [] this.temporaryValues = this.temporaryValues || [] + if (q.SELECT.from?.join && !q.SELECT.columns) { + throw new Error('CQN query using joins must specify the selected columns.') + } + const { limit, one, orderBy, expand, columns = ['*'], localized, count, parent } = q.SELECT const walkAlias = q => { From c5dafe8eae2b2ab56cbcf28b9198bca907dc2309 Mon Sep 17 00:00:00 2001 From: Johannes Vogel <31311694+johannes-vogel@users.noreply.github.com> Date: Wed, 20 Mar 2024 15:20:32 +0100 Subject: [PATCH 6/7] Update test/scenarios/bookshop/read.test.js Co-authored-by: Bob den Os <108393871+BobdenOs@users.noreply.github.com> --- test/scenarios/bookshop/read.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/scenarios/bookshop/read.test.js b/test/scenarios/bookshop/read.test.js index 6ab3a0f6b..2c8b2d22e 100644 --- a/test/scenarios/bookshop/read.test.js +++ b/test/scenarios/bookshop/read.test.js @@ -269,7 +269,7 @@ describe('Bookshop - Read', () => { }, } - expect(cds.db.run(query)).to.be.rejectedWith(/Ambiguous wildcard elements/) + return expect(cds.db.run(query)).to.be.rejectedWith(/Ambiguous wildcard elements/) }) it('joins without columns are rejected in general', async () => { From e0aa6a4de382fda047bdc6403684895ac403322f Mon Sep 17 00:00:00 2001 From: Johannes Vogel <31311694+johannes-vogel@users.noreply.github.com> Date: Wed, 20 Mar 2024 15:20:36 +0100 Subject: [PATCH 7/7] Update test/scenarios/bookshop/read.test.js Co-authored-by: Bob den Os <108393871+BobdenOs@users.noreply.github.com> --- test/scenarios/bookshop/read.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/scenarios/bookshop/read.test.js b/test/scenarios/bookshop/read.test.js index 2c8b2d22e..1d10cfadc 100644 --- a/test/scenarios/bookshop/read.test.js +++ b/test/scenarios/bookshop/read.test.js @@ -286,7 +286,7 @@ describe('Bookshop - Read', () => { }, } - expect(cds.db.run(query)).to.be.rejectedWith(/joins must specify the selected columns/) + return expect(cds.db.run(query)).to.be.rejectedWith(/joins must specify the selected columns/) }) test('Delete Book', async () => {