From 54d2efb00cfa4b5f188dc01bd350f3ccaca8986b Mon Sep 17 00:00:00 2001 From: Bob den Os <108393871+BobdenOs@users.noreply.github.com> Date: Mon, 22 Apr 2024 10:58:05 +0200 Subject: [PATCH 1/5] fix(hana): Align not found behavior in @cap-js/hana (#603) --- hana/lib/HANAService.js | 2 +- hana/lib/drivers/base.js | 6 ------ hana/lib/drivers/hana-client.js | 20 +++++++++++++++++--- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index 7f91dbb46..0400b43ed 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -153,7 +153,7 @@ class HANAService extends SQLService { // REVISIT: the runtime always expects that the count is preserved with .map, required for renaming in mocks return HANAService._arrayWithCount(rows, await this.count(query, rows)) } - return cqn.SELECT.one || query.SELECT.from.ref?.[0].cardinality?.max === 1 ? rows[0] || null : rows + return cqn.SELECT.one || query.SELECT.from.ref?.[0].cardinality?.max === 1 ? rows[0] : rows } async onINSERT({ query, data }) { diff --git a/hana/lib/drivers/base.js b/hana/lib/drivers/base.js index 3a7eb3009..d41e49fd9 100644 --- a/hana/lib/drivers/base.js +++ b/hana/lib/drivers/base.js @@ -31,12 +31,6 @@ class HANADriver { const stmt = await prep let changes = await prom(stmt, 'exec')(values) await this._sendStreams(stmt, streams) - // REVISIT: hana-client does not return any changes when doing an update with streams - // This causes the best assumption to be that the changes are one - // To get the correct information it is required to send a count with the update where clause - if (streams.length && changes === 0) { - changes = 1 - } return { changes } }, runBatch: async params => { diff --git a/hana/lib/drivers/hana-client.js b/hana/lib/drivers/hana-client.js index d63cd88d3..f63ee7fee 100644 --- a/hana/lib/drivers/hana-client.js +++ b/hana/lib/drivers/hana-client.js @@ -74,7 +74,7 @@ class HANAClientDriver extends driver { rsStreamsProm.resolve = resolve rsStreamsProm.reject = reject }) - rsStreams.catch(() => {}) + rsStreams.catch(() => { }) rs._rowPosition = -1 const _next = prom(rs, 'next') @@ -110,6 +110,20 @@ class HANAClientDriver extends driver { } } + ret.run = async params => { + const { values, streams } = this._extractStreams(params) + const stmt = await ret._prep + let changes = await prom(stmt, 'exec')(values) + await this._sendStreams(stmt, streams) + // REVISIT: hana-client does not return any changes when doing an update with streams + // This causes the best assumption to be that the changes are one + // To get the correct information it is required to send a count with the update where clause + if (streams.length && changes === 0) { + changes = 1 + } + return { changes } + } + ret.proc = async (data, outParameters) => { const stmt = await ret._prep const rows = await prom(stmt, 'execQuery')(data) @@ -161,7 +175,7 @@ class HANAClientDriver extends driver { } const resultSet = Array.isArray(rows) ? rows[0] : rows - + // merge table output params into scalar params const params = Array.isArray(outParameters) && outParameters.filter(md => !(md.PARAMETER_NAME in result)) if (params && params.length) { @@ -174,7 +188,7 @@ class HANAClientDriver extends driver { resultSet.nextResult() } } - + return result } From d8bac499cf57aee1ee7b39d168fdb3282f1ec01e Mon Sep 17 00:00:00 2001 From: Bob den Os <108393871+BobdenOs@users.noreply.github.com> Date: Mon, 22 Apr 2024 16:33:07 +0200 Subject: [PATCH 2/5] test: Reproduce exists path expression gap (#601) Co-authored-by: Patrice Bender --- db-service/lib/infer/index.js | 29 +++++++------ db-service/test/cds-infer/negative.test.js | 13 +++++- db-service/test/cqn4sql/expand.test.js | 5 ++- db-service/test/cqn4sql/where-exists.test.js | 6 +-- test/compliance/SELECT.test.js | 42 +++++++++++++++++-- test/compliance/UPDATE.test.js | 2 +- test/compliance/api.test.js | 10 ++--- .../resources/db/complex/associations.cds | 13 ++++++ .../db/complex/associationsUnmanaged.cds | 14 +++++++ .../compliance/resources/db/complex/index.cds | 13 +----- ...s.csv => complex.associations.Authors.csv} | 0 ...oks.csv => complex.associations.Books.csv} | 0 test/compliance/strictMode.test.js | 2 +- 13 files changed, 108 insertions(+), 41 deletions(-) create mode 100644 test/compliance/resources/db/complex/associations.cds create mode 100644 test/compliance/resources/db/complex/associationsUnmanaged.cds rename test/compliance/resources/db/data/{complex.Authors.csv => complex.associations.Authors.csv} (100%) rename test/compliance/resources/db/data/{complex.Books.csv => complex.associations.Books.csv} (100%) diff --git a/db-service/lib/infer/index.js b/db-service/lib/infer/index.js index 8bb11cb28..ed8c48bd6 100644 --- a/db-service/lib/infer/index.js +++ b/db-service/lib/infer/index.js @@ -180,13 +180,15 @@ function infer(originalQuery, model) { // only fk access in infix filter const nextStep = ref[1]?.id || ref[1] // no unmanaged assoc in infix filter path - if (!expandOrExists && e.on) - throw new Error( - `"${e.name}" in path "${arg.ref.map(idOnly).join('.')}" must not be an unmanaged association`, - ) + if (!expandOrExists && e.on) { + const err = `Unexpected unmanaged association “${e.name}” in filter expression of “${$baseLink.definition.name}”` + throw new Error(err) + } // no non-fk traversal in infix filter if (!expandOrExists && nextStep && !isForeignKeyOf(nextStep, e)) - throw new Error(`Only foreign keys of "${e.name}" can be accessed in infix filter`) + throw new Error( + `Only foreign keys of “${e.name}” can be accessed in infix filter, but found “${nextStep}”`, + ) } arg.$refLinks.push({ definition: e, target: definition }) // filter paths are flattened @@ -615,11 +617,10 @@ function infer(originalQuery, model) { if (!column.$refLinks[i].definition.target || danglingFilter) throw new Error('A filter can only be provided when navigating along associations') if (!column.expand) Object.defineProperty(column, 'isJoinRelevant', { value: true }) - // books[exists genre[code='A']].title --> column is join relevant but inner exists filter is not - let skipJoinsForFilter = inExists + let skipJoinsForFilter = false step.where.forEach(token => { if (token === 'exists') { - // no joins for infix filters along `exists ` + // books[exists genre[code='A']].title --> column is join relevant but inner exists filter is not skipJoinsForFilter = true } else if (token.ref || token.xpr) { inferQueryElement(token, false, column.$refLinks[i], { @@ -698,13 +699,15 @@ function infer(originalQuery, model) { // only fk access in infix filter const nextStep = column.ref[i + 1]?.id || column.ref[i + 1] // no unmanaged assoc in infix filter path - if (!inExists && assoc.on) - throw new Error( - `"${assoc.name}" in path "${column.ref.map(idOnly).join('.')}" must not be an unmanaged association`, - ) + if (!inExists && assoc.on) { + const err = `Unexpected unmanaged association “${assoc.name}” in filter expression of “${$baseLink.definition.name}”` + throw new Error(err) + } // no non-fk traversal in infix filter in non-exists path if (nextStep && !assoc.on && !isForeignKeyOf(nextStep, assoc)) - throw new Error(`Only foreign keys of "${assoc.name}" can be accessed in infix filter, not "${nextStep}"`) + throw new Error( + `Only foreign keys of “${assoc.name}” can be accessed in infix filter, but found “${nextStep}”`, + ) } } }) diff --git a/db-service/test/cds-infer/negative.test.js b/db-service/test/cds-infer/negative.test.js index b8e790180..0b67712be 100644 --- a/db-service/test/cds-infer/negative.test.js +++ b/db-service/test/cds-infer/negative.test.js @@ -392,9 +392,18 @@ describe('negative', () => { describe('infix filters', () => { it('rejects non fk traversal in infix filter in from', () => { expect(() => _inferred(CQL`SELECT from bookshop.Books[author.name = 'Kurt']`, model)).to.throw( - /Only foreign keys of "author" can be accessed in infix filter/, + /Only foreign keys of “author” can be accessed in infix filter, but found “name”/, ) }) + it('rejects non fk traversal in infix filter in where exists', () => { + let query = CQL`SELECT from bookshop.Books where exists author.books[author.name = 'John Doe']` + expect(() => _inferred(query)).to.throw(/Only foreign keys of “author” can be accessed in infix filter, but found “name”/,) // revisit: better error location ""bookshop.Books:author" + }) + it('rejects unmanaged traversal in infix filter in where exists', () => { + let query = CQL`SELECT from bookshop.Books where exists author.books[coAuthorUnmanaged.name = 'John Doe']` + expect(() => _inferred(query)).to.throw(/Unexpected unmanaged association “coAuthorUnmanaged” in filter expression of “books”/,) // revisit: better error location ""bookshop.Books:author" + }) + it('rejects non fk traversal in infix filter in column', () => { expect(() => _inferred( @@ -403,7 +412,7 @@ describe('negative', () => { }`, model, ), - ).to.throw(/Only foreign keys of "author" can be accessed in infix filter/) + ).to.throw(/Only foreign keys of “author” can be accessed in infix filter/) }) }) diff --git a/db-service/test/cqn4sql/expand.test.js b/db-service/test/cqn4sql/expand.test.js index 3710d5d85..6f1a2b56f 100644 --- a/db-service/test/cqn4sql/expand.test.js +++ b/db-service/test/cqn4sql/expand.test.js @@ -193,13 +193,14 @@ describe('Unfold expands on associations to special subselects', () => { // - they can return multiple rows it('rejects unmanaged association in infix filter of expand path', () => { expect(() => cqn4sql(CQL`SELECT from bookshop.Books { author[books.title = 'foo'] { name } }`, model)).to.throw( - /"books" in path "books.title" must not be an unmanaged association/, + /Unexpected unmanaged association “books” in filter expression of “author”/, ) + }) it('rejects non-fk access in infix filter of expand path', () => { expect(() => cqn4sql(CQL`SELECT from bookshop.EStrucSibling { self[sibling.struc1 = 'foo'] { ID } }`, model), - ).to.throw(/Only foreign keys of "sibling" can be accessed in infix filter/) + ).to.throw(/Only foreign keys of “sibling” can be accessed in infix filter/) }) it('unfold expand, one field', () => { const q = CQL`SELECT from bookshop.Books { diff --git a/db-service/test/cqn4sql/where-exists.test.js b/db-service/test/cqn4sql/where-exists.test.js index 83b2168c8..61e430714 100644 --- a/db-service/test/cqn4sql/where-exists.test.js +++ b/db-service/test/cqn4sql/where-exists.test.js @@ -218,7 +218,7 @@ describe('EXISTS predicate in where', () => { CQL`SELECT from bookshop.Authors { ID } WHERE EXISTS books[dedication.addressee.name = 'Hasso']`, model, ), - ).to.throw('Only foreign keys of "addressee" can be accessed in infix filter') + ).to.throw('Only foreign keys of “addressee” can be accessed in infix filter') }) it('MUST fail if following managed assoc in filter', () => { expect(() => @@ -226,7 +226,7 @@ describe('EXISTS predicate in where', () => { CQL`SELECT from bookshop.Authors { ID, books[dedication.addressee.name = 'Hasso'].dedication.addressee.name as Hasso }`, model, ), - ).to.throw('Only foreign keys of "addressee" can be accessed in infix filter') + ).to.throw('Only foreign keys of “addressee” can be accessed in infix filter') }) it('MUST handle simple where exists with multiple association and also with $self backlink', () => { @@ -719,7 +719,7 @@ describe('EXISTS predicate in infix filter', () => { ` expect(() => { cqn4sql(q, cds.compile.for.nodejs(JSON.parse(JSON.stringify(model)))) - }).to.throw(/Only foreign keys of "participant" can be accessed in infix filter/) + }).to.throw(/Only foreign keys of “participant” can be accessed in infix filter/) }) }) diff --git a/test/compliance/SELECT.test.js b/test/compliance/SELECT.test.js index 18551b710..06206a630 100644 --- a/test/compliance/SELECT.test.js +++ b/test/compliance/SELECT.test.js @@ -1,11 +1,13 @@ const assert = require('assert') +const chaiAsPromised = require('chai-as-promised'); const cds = require('../cds.js') // Set cds.root before requiring cds.Service as it resolves and caches package.json // Call default cds.test API describe('SELECT', () => { + // chai.use(chaiAsPromised) const { data, expect } = cds.test(__dirname + '/resources') data.autoIsolation(true) @@ -180,7 +182,7 @@ describe('SELECT', () => { const nulls = length => new Array(length).fill().map((_, i) => ({ as: `null${i}`, val: null })) const cqn = { SELECT: { - from: { ref: ['complex.Authors'] }, + from: { ref: ['complex.associations.Authors'] }, columns: [{ ref: ['ID'] }, { ref: ['name'] }, { ref: ['books'], expand: ['*', ...nulls(197)] }] }, } @@ -194,7 +196,7 @@ describe('SELECT', () => { const nulls = length => new Array(length).fill().map((_, i) => ({ as: `null${i}`, val: null })) const cqn = { SELECT: { - from: { ref: ['complex.Books'] }, + from: { ref: ['complex.associations.Books'] }, columns: [{ ref: ['ID'] }, { ref: ['title'] }, { ref: ['author'], expand: ['*', ...nulls(198)] }] }, } @@ -250,6 +252,40 @@ describe('SELECT', () => { assert.strictEqual(timestampMatches.length, 1, 'Ensure that the dateTime column matches the timestamp value') }) + test('exists path expression', async () => { + const cqn = { + SELECT: { + from: { ref: ["complex.associations.Books"] }, + where: [ + "exists", + { + ref: [ + "author", + { id: "books", where: [{ ref: ["author", "name"] }, "=", { val: "Emily" }] }] + } + ] + } + } + expect(cds.run(cqn)).to.eventually.be.rejectedWith('Only foreign keys of “author” can be accessed in infix filter, but found “name”'); + }) + + test('exists path expression (unmanaged)', async () => { + const cqn = { + SELECT: { + from: { ref: ["complex.associations.unmanaged.Books"] }, + where: [ + "exists", + { + ref: [ + "author", + { id: "books", where: [{ ref: ["author", "name"] }, "=", { val: "Emily" }] }] + } + ] + } + } + expect(cds.run(cqn)).to.eventually.be.rejectedWith('Unexpected unmanaged association “author” in filter expression of “books”'); + }) + test.skip('ref select', async () => { // Currently not possible as cqn4sql does not recognize where.ref.id: 'basic.projection.globals' as an external source const cqn = { @@ -454,7 +490,7 @@ describe('SELECT', () => { describe('count', () => { test('count is preserved with .map', async () => { - const query = SELECT.from('complex.Authors') + const query = SELECT.from('complex.associations.Authors') query.SELECT.count = true const result = await query assert.strictEqual(result.$count, 1) diff --git a/test/compliance/UPDATE.test.js b/test/compliance/UPDATE.test.js index 8c6648f0d..3dc82deac 100644 --- a/test/compliance/UPDATE.test.js +++ b/test/compliance/UPDATE.test.js @@ -1,5 +1,5 @@ const cds = require('../cds.js') -const Books = 'complex.Books' +const Books = 'complex.associations.Books' describe('UPDATE', () => { describe('entity', () => { diff --git a/test/compliance/api.test.js b/test/compliance/api.test.js index 73282c3ee..a66f912fc 100644 --- a/test/compliance/api.test.js +++ b/test/compliance/api.test.js @@ -7,12 +7,12 @@ describe('affected rows', () => { const { expect } = cds.test(__dirname + '/resources') test('Delete returns affected rows', async () => { - const affectedRows = await DELETE.from('complex.Books').where('ID = 4712') + const affectedRows = await DELETE.from('complex.associations.Books').where('ID = 4712') expect(affectedRows).to.be.eq(0) }) test('Insert returns affected rows and InsertResult', async () => { - const insert = INSERT.into('complex.Books').entries({ ID: 5 }) + const insert = INSERT.into('complex.associations.Books').entries({ ID: 5 }) const affectedRows = await cds.db.run(insert) // affectedRows is an InsertResult, so we need to do lose comparison here, as strict will not work due to InsertResult expect(affectedRows == 1).to.be.eq(true) @@ -21,14 +21,14 @@ const { expect } = cds.test(__dirname + '/resources') }) test('Update returns affected rows', async () => { - const { count } = await SELECT.one`count(*)`.from('complex.Books') + const { count } = await SELECT.one`count(*)`.from('complex.associations.Books') - const affectedRows = await UPDATE.entity('complex.Books').data({title: 'Book'}) + const affectedRows = await UPDATE.entity('complex.associations.Books').data({title: 'Book'}) expect(affectedRows).to.be.eq(count) }) test('Upsert returns affected rows', async () => { - const affectedRows = await UPSERT.into('complex.Books').entries({ID: 9999999, title: 'Book'}) + const affectedRows = await UPSERT.into('complex.associations.Books').entries({ID: 9999999, title: 'Book'}) expect(affectedRows).to.be.eq(1) }) }) diff --git a/test/compliance/resources/db/complex/associations.cds b/test/compliance/resources/db/complex/associations.cds new file mode 100644 index 000000000..6f5216285 --- /dev/null +++ b/test/compliance/resources/db/complex/associations.cds @@ -0,0 +1,13 @@ +namespace complex.associations; + +entity Books { + key ID : Integer; + title : String(111); + author : Association to Authors; +} + +entity Authors { + key ID : Integer; + name : String(111); + books : Association to many Books on books.author = $self; +} diff --git a/test/compliance/resources/db/complex/associationsUnmanaged.cds b/test/compliance/resources/db/complex/associationsUnmanaged.cds new file mode 100644 index 000000000..4919981c1 --- /dev/null +++ b/test/compliance/resources/db/complex/associationsUnmanaged.cds @@ -0,0 +1,14 @@ +namespace complex.associations.unmanaged; + +entity Books { + key ID : Integer; + title : String(111); + author_ID: Integer; + author : Association to Authors on author.ID = $self.author_ID; +} + +entity Authors { + key ID : Integer; + name : String(111); + books : Association to many Books on books.author = $self; +} diff --git a/test/compliance/resources/db/complex/index.cds b/test/compliance/resources/db/complex/index.cds index 2f24a820d..4f698be3d 100644 --- a/test/compliance/resources/db/complex/index.cds +++ b/test/compliance/resources/db/complex/index.cds @@ -1,13 +1,4 @@ namespace complex; -entity Books { - key ID : Integer; - title : String(111); - author : Association to Authors; -} - -entity Authors { - key ID : Integer; - name : String(111); - books : Association to many Books on books.author = $self; -} +using from './associations'; +using from './associationsUnmanaged'; diff --git a/test/compliance/resources/db/data/complex.Authors.csv b/test/compliance/resources/db/data/complex.associations.Authors.csv similarity index 100% rename from test/compliance/resources/db/data/complex.Authors.csv rename to test/compliance/resources/db/data/complex.associations.Authors.csv diff --git a/test/compliance/resources/db/data/complex.Books.csv b/test/compliance/resources/db/data/complex.associations.Books.csv similarity index 100% rename from test/compliance/resources/db/data/complex.Books.csv rename to test/compliance/resources/db/data/complex.associations.Books.csv diff --git a/test/compliance/strictMode.test.js b/test/compliance/strictMode.test.js index 7570d1af4..4c077ecc5 100644 --- a/test/compliance/strictMode.test.js +++ b/test/compliance/strictMode.test.js @@ -1,5 +1,5 @@ const cds = require('../cds.js') -const Books = 'complex.Books' +const Books = 'complex.associations.Books' describe('strict mode', () => { beforeAll(() => { From e0ca8d2e2a9d27670da3541c3cc7dfc8f084c2da Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 23 Apr 2024 08:20:53 +0000 Subject: [PATCH 3/5] chore(deps): update typescript-eslint monorepo to v7.7.1 (#609) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@typescript-eslint/eslint-plugin](https://typescript-eslint.io/packages/eslint-plugin) ([source](https://togithub.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin)) | [`7.7.0` -> `7.7.1`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/7.7.0/7.7.1) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@typescript-eslint%2feslint-plugin/7.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@typescript-eslint%2feslint-plugin/7.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@typescript-eslint%2feslint-plugin/7.7.0/7.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@typescript-eslint%2feslint-plugin/7.7.0/7.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [@typescript-eslint/parser](https://typescript-eslint.io/packages/parser) ([source](https://togithub.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser)) | [`7.7.0` -> `7.7.1`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/7.7.0/7.7.1) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@typescript-eslint%2fparser/7.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@typescript-eslint%2fparser/7.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@typescript-eslint%2fparser/7.7.0/7.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@typescript-eslint%2fparser/7.7.0/7.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes
typescript-eslint/typescript-eslint (@​typescript-eslint/eslint-plugin) ### [`v7.7.1`](https://togithub.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#771-2024-04-22) [Compare Source](https://togithub.com/typescript-eslint/typescript-eslint/compare/v7.7.0...v7.7.1) ##### 🩹 Fixes - **eslint-plugin:** \[no-unsafe-assignment] handle shorthand property assignment - **eslint-plugin:** \[explicit-function-return-type] fix checking wrong ancestor's return type - **eslint-plugin:** \[prefer-optional-chain] only look at left operand for `requireNullish` - **eslint-plugin:** \[no-for-in-array] refine report location - **eslint-plugin:** \[no-unnecessary-type-assertion] allow non-null assertion for void type ##### ❤️ Thank You - Abraham Guo - Kirk Waiblinger - YeonJuan You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
typescript-eslint/typescript-eslint (@​typescript-eslint/parser) ### [`v7.7.1`](https://togithub.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#771-2024-04-22) [Compare Source](https://togithub.com/typescript-eslint/typescript-eslint/compare/v7.7.0...v7.7.1) This was a version bump only for parser to align it with other projects, there were no code changes. You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
--- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/cap-js/cds-dbs). Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 84 +++++++++++++++++++++++------------------------ 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/package-lock.json b/package-lock.json index 87d00ebad..0d2e49707 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1474,16 +1474,16 @@ "dev": true }, "node_modules/@typescript-eslint/eslint-plugin": { - "version": "7.7.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-7.7.0.tgz", - "integrity": "sha512-GJWR0YnfrKnsRoluVO3PRb9r5aMZriiMMM/RHj5nnTrBy1/wIgk76XCtCKcnXGjpZQJQRFtGV9/0JJ6n30uwpQ==", + "version": "7.7.1", + "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-7.7.1.tgz", + "integrity": "sha512-KwfdWXJBOviaBVhxO3p5TJiLpNuh2iyXyjmWN0f1nU87pwyvfS0EmjC6ukQVYVFJd/K1+0NWGPDXiyEyQorn0Q==", "dev": true, "dependencies": { "@eslint-community/regexpp": "^4.10.0", - "@typescript-eslint/scope-manager": "7.7.0", - "@typescript-eslint/type-utils": "7.7.0", - "@typescript-eslint/utils": "7.7.0", - "@typescript-eslint/visitor-keys": "7.7.0", + "@typescript-eslint/scope-manager": "7.7.1", + "@typescript-eslint/type-utils": "7.7.1", + "@typescript-eslint/utils": "7.7.1", + "@typescript-eslint/visitor-keys": "7.7.1", "debug": "^4.3.4", "graphemer": "^1.4.0", "ignore": "^5.3.1", @@ -1542,15 +1542,15 @@ "dev": true }, "node_modules/@typescript-eslint/parser": { - "version": "7.7.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-7.7.0.tgz", - "integrity": "sha512-fNcDm3wSwVM8QYL4HKVBggdIPAy9Q41vcvC/GtDobw3c4ndVT3K6cqudUmjHPw8EAp4ufax0o58/xvWaP2FmTg==", + "version": "7.7.1", + "resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-7.7.1.tgz", + "integrity": "sha512-vmPzBOOtz48F6JAGVS/kZYk4EkXao6iGrD838sp1w3NQQC0W8ry/q641KU4PrG7AKNAf56NOcR8GOpH8l9FPCw==", "dev": true, "dependencies": { - "@typescript-eslint/scope-manager": "7.7.0", - "@typescript-eslint/types": "7.7.0", - "@typescript-eslint/typescript-estree": "7.7.0", - "@typescript-eslint/visitor-keys": "7.7.0", + "@typescript-eslint/scope-manager": "7.7.1", + "@typescript-eslint/types": "7.7.1", + "@typescript-eslint/typescript-estree": "7.7.1", + "@typescript-eslint/visitor-keys": "7.7.1", "debug": "^4.3.4" }, "engines": { @@ -1570,13 +1570,13 @@ } }, "node_modules/@typescript-eslint/scope-manager": { - "version": "7.7.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-7.7.0.tgz", - "integrity": "sha512-/8INDn0YLInbe9Wt7dK4cXLDYp0fNHP5xKLHvZl3mOT5X17rK/YShXaiNmorl+/U4VKCVIjJnx4Ri5b0y+HClw==", + "version": "7.7.1", + "resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-7.7.1.tgz", + "integrity": "sha512-PytBif2SF+9SpEUKynYn5g1RHFddJUcyynGpztX3l/ik7KmZEv19WCMhUBkHXPU9es/VWGD3/zg3wg90+Dh2rA==", "dev": true, "dependencies": { - "@typescript-eslint/types": "7.7.0", - "@typescript-eslint/visitor-keys": "7.7.0" + "@typescript-eslint/types": "7.7.1", + "@typescript-eslint/visitor-keys": "7.7.1" }, "engines": { "node": "^18.18.0 || >=20.0.0" @@ -1587,13 +1587,13 @@ } }, "node_modules/@typescript-eslint/type-utils": { - "version": "7.7.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/type-utils/-/type-utils-7.7.0.tgz", - "integrity": "sha512-bOp3ejoRYrhAlnT/bozNQi3nio9tIgv3U5C0mVDdZC7cpcQEDZXvq8inrHYghLVwuNABRqrMW5tzAv88Vy77Sg==", + "version": "7.7.1", + "resolved": "https://registry.npmjs.org/@typescript-eslint/type-utils/-/type-utils-7.7.1.tgz", + "integrity": "sha512-ZksJLW3WF7o75zaBPScdW1Gbkwhd/lyeXGf1kQCxJaOeITscoSl0MjynVvCzuV5boUz/3fOI06Lz8La55mu29Q==", "dev": true, "dependencies": { - "@typescript-eslint/typescript-estree": "7.7.0", - "@typescript-eslint/utils": "7.7.0", + "@typescript-eslint/typescript-estree": "7.7.1", + "@typescript-eslint/utils": "7.7.1", "debug": "^4.3.4", "ts-api-utils": "^1.3.0" }, @@ -1614,9 +1614,9 @@ } }, "node_modules/@typescript-eslint/types": { - "version": "7.7.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-7.7.0.tgz", - "integrity": "sha512-G01YPZ1Bd2hn+KPpIbrAhEWOn5lQBrjxkzHkWvP6NucMXFtfXoevK82hzQdpfuQYuhkvFDeQYbzXCjR1z9Z03w==", + "version": "7.7.1", + "resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-7.7.1.tgz", + "integrity": "sha512-AmPmnGW1ZLTpWa+/2omPrPfR7BcbUU4oha5VIbSbS1a1Tv966bklvLNXxp3mrbc+P2j4MNOTfDffNsk4o0c6/w==", "dev": true, "engines": { "node": "^18.18.0 || >=20.0.0" @@ -1627,13 +1627,13 @@ } }, "node_modules/@typescript-eslint/typescript-estree": { - "version": "7.7.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-7.7.0.tgz", - "integrity": "sha512-8p71HQPE6CbxIBy2kWHqM1KGrC07pk6RJn40n0DSc6bMOBBREZxSDJ+BmRzc8B5OdaMh1ty3mkuWRg4sCFiDQQ==", + "version": "7.7.1", + "resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-7.7.1.tgz", + "integrity": "sha512-CXe0JHCXru8Fa36dteXqmH2YxngKJjkQLjxzoj6LYwzZ7qZvgsLSc+eqItCrqIop8Vl2UKoAi0StVWu97FQZIQ==", "dev": true, "dependencies": { - "@typescript-eslint/types": "7.7.0", - "@typescript-eslint/visitor-keys": "7.7.0", + "@typescript-eslint/types": "7.7.1", + "@typescript-eslint/visitor-keys": "7.7.1", "debug": "^4.3.4", "globby": "^11.1.0", "is-glob": "^4.0.3", @@ -1712,17 +1712,17 @@ "dev": true }, "node_modules/@typescript-eslint/utils": { - "version": "7.7.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-7.7.0.tgz", - "integrity": "sha512-LKGAXMPQs8U/zMRFXDZOzmMKgFv3COlxUQ+2NMPhbqgVm6R1w+nU1i4836Pmxu9jZAuIeyySNrN/6Rc657ggig==", + "version": "7.7.1", + "resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-7.7.1.tgz", + "integrity": "sha512-QUvBxPEaBXf41ZBbaidKICgVL8Hin0p6prQDu6bbetWo39BKbWJxRsErOzMNT1rXvTll+J7ChrbmMCXM9rsvOQ==", "dev": true, "dependencies": { "@eslint-community/eslint-utils": "^4.4.0", "@types/json-schema": "^7.0.15", "@types/semver": "^7.5.8", - "@typescript-eslint/scope-manager": "7.7.0", - "@typescript-eslint/types": "7.7.0", - "@typescript-eslint/typescript-estree": "7.7.0", + "@typescript-eslint/scope-manager": "7.7.1", + "@typescript-eslint/types": "7.7.1", + "@typescript-eslint/typescript-estree": "7.7.1", "semver": "^7.6.0" }, "engines": { @@ -1770,12 +1770,12 @@ "dev": true }, "node_modules/@typescript-eslint/visitor-keys": { - "version": "7.7.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-7.7.0.tgz", - "integrity": "sha512-h0WHOj8MhdhY8YWkzIF30R379y0NqyOHExI9N9KCzvmu05EgG4FumeYa3ccfKUSphyWkWQE1ybVrgz/Pbam6YA==", + "version": "7.7.1", + "resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-7.7.1.tgz", + "integrity": "sha512-gBL3Eq25uADw1LQ9kVpf3hRM+DWzs0uZknHYK3hq4jcTPqVCClHGDnB6UUUV2SFeBeA4KWHWbbLqmbGcZ4FYbw==", "dev": true, "dependencies": { - "@typescript-eslint/types": "7.7.0", + "@typescript-eslint/types": "7.7.1", "eslint-visitor-keys": "^3.4.3" }, "engines": { From e7ec086594df77ff51fc5d2690512211380ed2e6 Mon Sep 17 00:00:00 2001 From: Patrice Bender Date: Tue, 23 Apr 2024 15:53:19 +0200 Subject: [PATCH 4/5] chore: fix eslint warnings (#613) --- db-service/test/tsc/typescript.ts | 7 ++++--- eslint.config.js | 4 ++-- test/compliance/SELECT.test.js | 2 -- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/db-service/test/tsc/typescript.ts b/db-service/test/tsc/typescript.ts index 5ef9a0b4c..8a0bbde4b 100644 --- a/db-service/test/tsc/typescript.ts +++ b/db-service/test/tsc/typescript.ts @@ -118,7 +118,7 @@ export class TestCQN2SQL extends CQN2SQL { super(context) } - SELECT(cqn: any): string { + SELECT(cqn: JSON): string { cqn return '' } @@ -173,10 +173,10 @@ type sourceElementResult> { - from = (x: SRC): SELECT => { + from = (_x: SRC): SELECT => { return this } - columns = >(x: sourceElementRef[]): SELECT => { + columns = >(_x: sourceElementRef[]): SELECT => { return this } then = >( @@ -185,6 +185,7 @@ class SELECT> { ): void => { try { // This is not a real solution, but it would work in javascript + // eslint-disable-next-line @typescript-eslint/no-explicit-any resolve({ ID: 1 } as any) } catch (e) { reject(new Error('oops')) diff --git a/eslint.config.js b/eslint.config.js index 93cefdcce..2cfc2d67d 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -49,14 +49,14 @@ module.exports = [ }, rules: { ...eslint_ts.configs.recommended.rules, - '@typescript-eslint/no-unused-vars': ['warn'], + '@typescript-eslint/no-unused-vars': ['warn', { argsIgnorePattern: '^_' }], '@typescript-eslint/no-explicit-any': ['warn'], }, }, { files: ['**/hana/**/*.js'], rules: { - 'no-console': 'warn', + 'no-console': 'off', }, }, ] diff --git a/test/compliance/SELECT.test.js b/test/compliance/SELECT.test.js index 06206a630..1bd02bc75 100644 --- a/test/compliance/SELECT.test.js +++ b/test/compliance/SELECT.test.js @@ -1,6 +1,4 @@ const assert = require('assert') - -const chaiAsPromised = require('chai-as-promised'); const cds = require('../cds.js') // Set cds.root before requiring cds.Service as it resolves and caches package.json From e1a77119f0a5241cfe4f50a37a473f2325ba5bde Mon Sep 17 00:00:00 2001 From: Patrice Bender Date: Tue, 23 Apr 2024 16:06:04 +0200 Subject: [PATCH 5/5] fix: assign artificial alias if selecting from anonymous subquery (#608) This is necessary if we expose an association in the inner query and traverse it in the outer query: ```cds select from (select from Books) { author.name } ``` to come up with a proper join node, we must add a table alias around the subquery in from. I came up with `__select__` as artifical name. To make it always unique, I request a unique alias from the subquery based on `__select__`, the subquery knows all outer aliases and is able to calculate a universally unique alias. bugs found in #590 --- db-service/lib/cqn4sql.js | 6 +- db-service/lib/infer/index.js | 6 +- db-service/lib/infer/join-tree.js | 2 + db-service/test/bookshop/db/booksWithExpr.cds | 7 ++ .../test/cqn4sql/calculated-elements.test.js | 49 +++++++++++ db-service/test/cqn4sql/table-alias.test.js | 87 ++++++++++++++----- 6 files changed, 133 insertions(+), 24 deletions(-) diff --git a/db-service/lib/cqn4sql.js b/db-service/lib/cqn4sql.js index 5efc72ece..edc5e42a8 100644 --- a/db-service/lib/cqn4sql.js +++ b/db-service/lib/cqn4sql.js @@ -1576,9 +1576,13 @@ function cqn4sql(originalQuery, model) { return { transformedFrom } } else if (from.SELECT) { transformedFrom = transformSubquery(from) - if (from.as) + if (from.as) { // preserve explicit TA transformedFrom.as = from.as + } else { + // select from anonymous query, use artificial alias + transformedFrom.as = Object.keys(originalQuery.sources)[0] + } return { transformedFrom } } else { return _transformFrom() diff --git a/db-service/lib/infer/index.js b/db-service/lib/infer/index.js index ed8c48bd6..32fccffb1 100644 --- a/db-service/lib/infer/index.js +++ b/db-service/lib/infer/index.js @@ -123,8 +123,10 @@ function infer(originalQuery, model) { } else if (from.args) { from.args.forEach(a => inferTarget(a, querySources)) } else if (from.SELECT) { - infer(from, model) // we need the .elements in the sources - querySources[from.as || ''] = { definition: from } + const subqueryInFrom = infer(from, model) // we need the .elements in the sources + // if no explicit alias is provided, we make up one + const subqueryAlias = from.as || subqueryInFrom.joinTree.addNextAvailableTableAlias('__select__', subqueryInFrom.outerQueries) + querySources[subqueryAlias] = { definition: from } } else if (typeof from === 'string') { // TODO: Create unique alias, what about duplicates? const definition = getDefinition(from) || cds.error`"${from}" not found in the definitions of your model` diff --git a/db-service/lib/infer/join-tree.js b/db-service/lib/infer/join-tree.js index 6d7ce7361..82b1bfc28 100644 --- a/db-service/lib/infer/join-tree.js +++ b/db-service/lib/infer/join-tree.js @@ -176,6 +176,8 @@ class JoinTree { i += 1 // skip first step which is table alias } + // if no root node was found, the column is selected from a subquery + if(!node) return while (i < col.ref.length) { const step = col.ref[i] const { where, args } = step diff --git a/db-service/test/bookshop/db/booksWithExpr.cds b/db-service/test/bookshop/db/booksWithExpr.cds index c21ab9caf..3fbe22032 100644 --- a/db-service/test/bookshop/db/booksWithExpr.cds +++ b/db-service/test/bookshop/db/booksWithExpr.cds @@ -84,3 +84,10 @@ entity LBooks { width : Decimal; area : Decimal = length * width; } + +entity Simple { + key ID: Integer; + name: String; + my: Association to Simple; + myName: String = my.name; +} diff --git a/db-service/test/cqn4sql/calculated-elements.test.js b/db-service/test/cqn4sql/calculated-elements.test.js index 1abb0c7e5..421bbefcc 100644 --- a/db-service/test/cqn4sql/calculated-elements.test.js +++ b/db-service/test/cqn4sql/calculated-elements.test.js @@ -509,6 +509,55 @@ describe('Unfolding calculated elements in select list', () => { expect(JSON.parse(JSON.stringify(query))).to.deep.equal(expected) }) + it('wildcard select from subquery', () => { + let query = cqn4sql( + CQL`SELECT from ( SELECT FROM booksCalc.Simple { * } )`, + model, + ) + const expected = CQL` + SELECT from ( + SELECT from booksCalc.Simple as Simple + left join booksCalc.Simple as my on my.ID = Simple.my_ID + { + Simple.ID, + Simple.name, + Simple.my_ID, + my.name as myName + } + ) as __select__ { + __select__.ID, + __select__.name, + __select__.my_ID, + __select__.myName + } + ` + expect(JSON.parse(JSON.stringify(query))).to.deep.equal(expected) + }) + + it('wildcard select from subquery + join relevant path expression', () => { + let query = cqn4sql( + CQL`SELECT from ( SELECT FROM booksCalc.Simple { * } ) { + my.name as otherName + }`, + model, + ) + const expected = CQL` + SELECT from ( + SELECT from booksCalc.Simple as Simple + left join booksCalc.Simple as my2 on my2.ID = Simple.my_ID + { + Simple.ID, + Simple.name, + Simple.my_ID, + my2.name as myName + } + ) as __select__ left join booksCalc.Simple as my on my.ID = __select__.my_ID { + my.name as otherName + } + ` + expect(JSON.parse(JSON.stringify(query))).to.deep.equal(expected) + }) + it('replacement for calculated element is considered for wildcard expansion', () => { let query = cqn4sql( CQL`SELECT from booksCalc.Books { *, volume as ctitle } excluding { length, width, height, stock, price, youngAuthorName }`, diff --git a/db-service/test/cqn4sql/table-alias.test.js b/db-service/test/cqn4sql/table-alias.test.js index a2d343727..29df94fce 100644 --- a/db-service/test/cqn4sql/table-alias.test.js +++ b/db-service/test/cqn4sql/table-alias.test.js @@ -18,9 +18,59 @@ describe('table alias access', () => { expect(query).to.deep.equal(CQL`SELECT from bookshop.Books as Books { Books.ID }`) }) - it('omits alias for anonymous query which selects from other query', () => { + it('creates unique alias for anonymous query which selects from other query', () => { let query = cqn4sql(CQL`SELECT from (SELECT from bookshop.Books { ID } )`, model) - expect(query).to.deep.equal(CQL`SELECT from (SELECT from bookshop.Books as Books { Books.ID }) { ID }`) + expect(query).to.deep.equal( + CQL`SELECT from (SELECT from bookshop.Books as Books { Books.ID }) as __select__ { __select__.ID }`, + ) + }) + + it('the unique alias for anonymous query does not collide with user provided aliases', () => { + let query = cqn4sql(CQL`SELECT from (SELECT from bookshop.Books as __select__ { ID } )`, model) + expect(query).to.deep.equal( + CQL`SELECT from (SELECT from bookshop.Books as __select__ { __select__.ID }) as __select__2 { __select__2.ID }`, + ) + }) + it('the unique alias for anonymous query does not collide with user provided aliases in case of joins', () => { + let query = cqn4sql( + CQL`SELECT from (SELECT from bookshop.Books as __select__ { ID, author } ) { author.name }`, + model, + ) + expect(query).to.deep.equal(CQL` + SELECT from ( + SELECT from bookshop.Books as __select__ { __select__.ID, __select__.author_ID } + ) as __select__2 left join bookshop.Authors as author on author.ID = __select__2.author_ID + { + author.name as author_name + }`) + }) + + it('the unique alias for anonymous query does not collide with user provided aliases nested', () => { + // author association bubbles up to the top query where the join finally is done + // --> note that the most outer query uses user defined __select__ alias + let query = cqn4sql( + CQL` + SELECT from ( + SELECT from ( + SELECT from bookshop.Books { ID, author } + ) + ) as __select__ + { + __select__.author.name + }`, + model, + ) + expect(query).to.deep.equal( + CQL` + SELECT from ( + SELECT from ( + SELECT from bookshop.Books as Books { Books.ID, Books.author_ID } + ) as __select__2 { __select__2.ID, __select__2.author_ID } + ) as __select__ left join bookshop.Authors as author on author.ID = __select__.author_ID + { + author.name as author_name + }`, + ) }) it('preserves table alias at field access', () => { @@ -444,16 +494,12 @@ describe('table alias access', () => { } ORDER BY Books.title, Books.title`) }) - it('dont try to prepend table alias if we select from anonymous subquery', async () => { + it('prepend artificial table alias if we select from anonymous subquery', async () => { const subquery = SELECT.localized.from('bookshop.SimpleBook').orderBy('title') - const query = SELECT.localized - .columns('ID', 'title', 'author') - .from(subquery) - .orderBy('title') - .groupBy('title') - + const query = SELECT.localized.columns('ID', 'title', 'author').from(subquery).orderBy('title').groupBy('title') + query.SELECT.count = true - + const res = cqn4sql(query, model) const expected = CQL` @@ -464,14 +510,14 @@ describe('table alias access', () => { SimpleBook.author_ID from bookshop.SimpleBook as SimpleBook order by SimpleBook.title - ) + ) __select__ { - ID, - title, - author_ID + __select__.ID, + __select__.title, + __select__.author_ID } - group by title - order by title + group by __select__.title + order by __select__.title ` expect(JSON.parse(JSON.stringify(res))).to.deep.equal(expected) }) @@ -561,7 +607,6 @@ describe('table alias access', () => { { SimpleBook.ID, SimpleBook.title, SimpleBook.author_ID } order by author.name` expect(query).to.deep.equal(expected) }) - }) describe('replace usage of implicit aliases in subqueries', () => { @@ -836,7 +881,7 @@ describe('table alias access', () => { }`, ) }) - it('no alias for function args or expressions on top of anonymous subquery', () => { + it('prepends unique alias for function args or expressions on top of anonymous subquery', () => { let query = cqn4sql( CQL`SELECT from ( SELECT from bookshop.Orders ) { sum(ID) as foo, @@ -849,9 +894,9 @@ describe('table alias access', () => { SELECT from bookshop.Orders as Orders { Orders.ID } - ) { - sum(ID) as foo, - ID + 42 as anotherFoo + ) as __select__ { + sum(__select__.ID) as foo, + __select__.ID + 42 as anotherFoo }`, ) })