From 089e0c2b01547c407e5ebbccaaaad0fba2afe902 Mon Sep 17 00:00:00 2001 From: Patrice Bender Date: Thu, 28 Nov 2024 13:09:36 +0100 Subject: [PATCH 1/4] fix: wildcard expand with aggregation --- db-service/lib/cqn4sql.js | 15 +++++++++++---- db-service/test/cqn4sql/expand.test.js | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/db-service/lib/cqn4sql.js b/db-service/lib/cqn4sql.js index 57b68f7a6..33b8148e9 100644 --- a/db-service/lib/cqn4sql.js +++ b/db-service/lib/cqn4sql.js @@ -873,6 +873,16 @@ function cqn4sql(originalQuery, model) { // to be attached to dummy query const elements = {} + const wildcardIndex = column.expand.findIndex(e => e === '*') + if (wildcardIndex !== -1) { + // replace wildcard with explicit refs + const dummy = { SELECT: { from: { ref: [column.element.target] } } } + const { + SELECT: { columns }, + } = cqn4sql(dummy, model) + columns.forEach(c => c.ref.splice(0, 1)) + column.expand.splice(wildcardIndex, 1, ...columns) + } const expandedColumns = column.expand.flatMap(expand => { const fullRef = [...baseRef, ...expand.ref] @@ -2202,10 +2212,7 @@ function cqn4sql(originalQuery, model) { const xpr = search const searchFunc = { func: 'search', - args: [ - { list: searchIn }, - xpr.length === 1 && 'val' in xpr[0] ? xpr[0] : { xpr }, - ], + args: [{ list: searchIn }, xpr.length === 1 && 'val' in xpr[0] ? xpr[0] : { xpr }], } return searchFunc } else { diff --git a/db-service/test/cqn4sql/expand.test.js b/db-service/test/cqn4sql/expand.test.js index 40436f3b9..c5f932859 100644 --- a/db-service/test/cqn4sql/expand.test.js +++ b/db-service/test/cqn4sql/expand.test.js @@ -1050,6 +1050,23 @@ describe('Expands with aggregations are special', () => { const res = cqn4sql(q, model) expect(JSON.parse(JSON.stringify(res))).to.deep.equal(qx) }) + + it('simple aggregation with wildcard expand', () => { + const q = CQL`SELECT from bookshop.TestPublisher { + ID, + publisher { * } + } group by ID, publisher.structuredKey_ID, publisher.title` + + const qx = CQL`SELECT from bookshop.TestPublisher as TestPublisher + left join bookshop.Publisher as publisher on publisher.structuredKey_ID = TestPublisher.publisher_structuredKey_ID { + TestPublisher.ID, + (SELECT from DUMMY { TestPublisher.publisher_structuredKey_ID as structuredKey_ID, publisher.title as title }) as publisher + } group by TestPublisher.ID, TestPublisher.publisher_structuredKey_ID, publisher.title` + qx.SELECT.columns[1].SELECT.from = null + // the key is not flat in the model so we use a flat csn for this test + const res = cqn4sql(q, cds.compile.for.nodejs(model)) + expect(JSON.parse(JSON.stringify(res))).to.deep.equal(qx) + }) it('aggregation with structure', () => { const q = CQL`SELECT from bookshop.Authors as Authors { ID, From 820d35cfc929c4ab3c3d148841a993a646038336 Mon Sep 17 00:00:00 2001 From: Patrice Bender Date: Thu, 28 Nov 2024 13:59:02 +0100 Subject: [PATCH 2/4] improve test --- db-service/lib/cqn4sql.js | 1 + db-service/test/cqn4sql/expand.test.js | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/db-service/lib/cqn4sql.js b/db-service/lib/cqn4sql.js index 33b8148e9..bf0e48189 100644 --- a/db-service/lib/cqn4sql.js +++ b/db-service/lib/cqn4sql.js @@ -884,6 +884,7 @@ function cqn4sql(originalQuery, model) { column.expand.splice(wildcardIndex, 1, ...columns) } const expandedColumns = column.expand.flatMap(expand => { + if(!expand.ref) return expand const fullRef = [...baseRef, ...expand.ref] if (expand.expand) { diff --git a/db-service/test/cqn4sql/expand.test.js b/db-service/test/cqn4sql/expand.test.js index c5f932859..51502b031 100644 --- a/db-service/test/cqn4sql/expand.test.js +++ b/db-service/test/cqn4sql/expand.test.js @@ -1050,17 +1050,32 @@ describe('Expands with aggregations are special', () => { const res = cqn4sql(q, model) expect(JSON.parse(JSON.stringify(res))).to.deep.equal(qx) }) + it.skip('simple aggregation expand ref wrapped in func', () => { + // TODO: how to detect the nested ref? + const q = CQL`SELECT from bookshop.Books { + ID, + Books.author { toLower(name) as lower } + } group by author.name` + + const qx = CQL`SELECT from bookshop.Books as Books left join bookshop.Authors as author on author.ID = Books.author_ID { + Books.ID, + (SELECT from DUMMY { toLower(author.name) as name }) as author + } group by author.name` + qx.SELECT.columns[1].SELECT.from = null + const res = cqn4sql(q, model) + expect(JSON.parse(JSON.stringify(res))).to.deep.equal(qx) + }) it('simple aggregation with wildcard expand', () => { const q = CQL`SELECT from bookshop.TestPublisher { ID, - publisher { * } + publisher { *, 1 + 1 as two } } group by ID, publisher.structuredKey_ID, publisher.title` const qx = CQL`SELECT from bookshop.TestPublisher as TestPublisher left join bookshop.Publisher as publisher on publisher.structuredKey_ID = TestPublisher.publisher_structuredKey_ID { TestPublisher.ID, - (SELECT from DUMMY { TestPublisher.publisher_structuredKey_ID as structuredKey_ID, publisher.title as title }) as publisher + (SELECT from DUMMY { TestPublisher.publisher_structuredKey_ID as structuredKey_ID, publisher.title as title, 1 + 1 as two }) as publisher } group by TestPublisher.ID, TestPublisher.publisher_structuredKey_ID, publisher.title` qx.SELECT.columns[1].SELECT.from = null // the key is not flat in the model so we use a flat csn for this test From f098bd91d5fa25fba17b33491e5476190e9ff094 Mon Sep 17 00:00:00 2001 From: Patrice Bender Date: Fri, 29 Nov 2024 12:42:13 +0100 Subject: [PATCH 3/4] ignore expand with wildcard for group by queries --- db-service/lib/cqn4sql.js | 17 ++++++----------- db-service/test/cqn4sql/expand.test.js | 9 +++------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/db-service/lib/cqn4sql.js b/db-service/lib/cqn4sql.js index 4186b2219..77f57f5c7 100644 --- a/db-service/lib/cqn4sql.js +++ b/db-service/lib/cqn4sql.js @@ -408,6 +408,7 @@ function cqn4sql(originalQuery, model) { const last = $refLinks?.[$refLinks.length - 1] if (last && !last.skipExpand && last.definition.isAssociation) { const expandedSubqueryColumn = expandColumn(col) + if (!expandedSubqueryColumn) return [] setElementOnColumns(expandedSubqueryColumn, col.element) res.push(expandedSubqueryColumn) } else if (!last?.skipExpand) { @@ -863,7 +864,7 @@ function cqn4sql(originalQuery, model) { * @param {Object} column - To expand. * @param {Array} baseRef - The base reference for the expanded column. * @param {string} subqueryAlias - The alias of the `expand` subquery column. - * @returns {Object} - The subquery object. + * @returns {Object} - The subquery object or null if the expand has a wildcard. * @throws {Error} - If one of the `ref`s in the `column.expand` is not part of the GROUP BY clause. */ function _subqueryForGroupBy(column, baseRef, subqueryAlias) { @@ -875,16 +876,11 @@ function cqn4sql(originalQuery, model) { const elements = {} const wildcardIndex = column.expand.findIndex(e => e === '*') if (wildcardIndex !== -1) { - // replace wildcard with explicit refs - const dummy = { SELECT: { from: { ref: [column.element.target] } } } - const { - SELECT: { columns }, - } = cqn4sql(dummy, model) - columns.forEach(c => c.ref.splice(0, 1)) - column.expand.splice(wildcardIndex, 1, ...columns) + // expand with wildcard vanishes as expand is part of the group by (OData $apply + $expand) + return null } const expandedColumns = column.expand.flatMap(expand => { - if(!expand.ref) return expand + if (!expand.ref) return expand const fullRef = [...baseRef, ...expand.ref] if (expand.expand) { @@ -1415,8 +1411,7 @@ function cqn4sql(originalQuery, model) { if (list.every(e => e.val)) // no need for transformation transformedTokenStream.push({ list }) - else - transformedTokenStream.push({ list: getTransformedTokenStream(list, $baseLink) }) + else transformedTokenStream.push({ list: getTransformedTokenStream(list, $baseLink) }) } } else if (tokenStream.length === 1 && token.val && $baseLink) { // infix filter - OData variant w/o mentioning key --> flatten out and compare each leaf to token.val diff --git a/db-service/test/cqn4sql/expand.test.js b/db-service/test/cqn4sql/expand.test.js index 51502b031..82e343178 100644 --- a/db-service/test/cqn4sql/expand.test.js +++ b/db-service/test/cqn4sql/expand.test.js @@ -1066,18 +1066,15 @@ describe('Expands with aggregations are special', () => { expect(JSON.parse(JSON.stringify(res))).to.deep.equal(qx) }) - it('simple aggregation with wildcard expand', () => { + it('expand vanishes if wildcard', () => { const q = CQL`SELECT from bookshop.TestPublisher { - ID, - publisher { *, 1 + 1 as two } + ID } group by ID, publisher.structuredKey_ID, publisher.title` const qx = CQL`SELECT from bookshop.TestPublisher as TestPublisher left join bookshop.Publisher as publisher on publisher.structuredKey_ID = TestPublisher.publisher_structuredKey_ID { - TestPublisher.ID, - (SELECT from DUMMY { TestPublisher.publisher_structuredKey_ID as structuredKey_ID, publisher.title as title, 1 + 1 as two }) as publisher + TestPublisher.ID } group by TestPublisher.ID, TestPublisher.publisher_structuredKey_ID, publisher.title` - qx.SELECT.columns[1].SELECT.from = null // the key is not flat in the model so we use a flat csn for this test const res = cqn4sql(q, cds.compile.for.nodejs(model)) expect(JSON.parse(JSON.stringify(res))).to.deep.equal(qx) From 9a3c395403ab145c991d681132823ff2677d8991 Mon Sep 17 00:00:00 2001 From: Patrice Bender Date: Fri, 29 Nov 2024 12:48:46 +0100 Subject: [PATCH 4/4] better test name --- db-service/test/cqn4sql/expand.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db-service/test/cqn4sql/expand.test.js b/db-service/test/cqn4sql/expand.test.js index 82e343178..2d14ba86a 100644 --- a/db-service/test/cqn4sql/expand.test.js +++ b/db-service/test/cqn4sql/expand.test.js @@ -1066,7 +1066,7 @@ describe('Expands with aggregations are special', () => { expect(JSON.parse(JSON.stringify(res))).to.deep.equal(qx) }) - it('expand vanishes if wildcard', () => { + it('wildcard expand vanishes for aggregations', () => { const q = CQL`SELECT from bookshop.TestPublisher { ID } group by ID, publisher.structuredKey_ID, publisher.title`