Skip to content

Commit

Permalink
fix: optimize foreign key access for expand with aggregations (#734)
Browse files Browse the repository at this point in the history
this is another follow up for #708

-> if the expand column was a foreign key access, the column was not
properly re-written which resulted in the reported error.

we now make use of the common functionality which ensure proper column
re-write. Along the way, support for structured columns was added as
well.
  • Loading branch information
patricebender authored Jul 8, 2024
1 parent e9e9461 commit 77b7978
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 21 deletions.
38 changes: 17 additions & 21 deletions db-service/lib/cqn4sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ const { pseudos } = require('./infer/pseudos')
*/
function cqn4sql(originalQuery, model) {
let inferred = typeof originalQuery === 'string' ? cds.parse.cql(originalQuery) : cds.ql.clone(originalQuery)
const hasCustomJoins = originalQuery.SELECT?.from.args && (!originalQuery.joinTree || originalQuery.joinTree.isInitial)
const hasCustomJoins =
originalQuery.SELECT?.from.args && (!originalQuery.joinTree || originalQuery.joinTree.isInitial)

if (!hasCustomJoins && inferred.SELECT?.search) {
// we need an instance of query because the elements of the query are needed for the calculation of the search columns
Expand Down Expand Up @@ -226,7 +227,7 @@ function cqn4sql(originalQuery, model) {
*/
function transformSearch(searchTerm) {
let prop = 'where'

// if the query is grouped and the queries columns contain an aggregate function,
// we must put the search term into the `having` clause, as the search expression
// is defined on the aggregated result, not on the individual rows
Expand Down Expand Up @@ -878,7 +879,7 @@ function cqn4sql(originalQuery, model) {

// to be attached to dummy query
const elements = {}
const expandedColumns = column.expand.map(expand => {
const expandedColumns = column.expand.flatMap(expand => {
const fullRef = [...baseRef, ...expand.ref]

if (expand.expand) {
Expand All @@ -898,13 +899,11 @@ function cqn4sql(originalQuery, model) {
if (expand.as) {
columnCopy.as = expand.as
}
if (columnCopy.isJoinRelevant) {
const tableAlias = getQuerySourceName(columnCopy)
const name = calculateElementName(columnCopy)
columnCopy.ref = [tableAlias, name]
}
elements[expand.as || expand.ref.map(idOnly).join('_')] = columnCopy.$refLinks.at(-1).definition
return columnCopy
const res = getFlatColumnsFor(columnCopy, { tableAlias: getQuerySourceName(columnCopy) })
res.forEach(c => {
elements[c.as || c.ref.at(-1)] = c.element
})
return res
})

const SELECT = {
Expand Down Expand Up @@ -935,15 +934,6 @@ function cqn4sql(originalQuery, model) {
if (isCalculatedOnRead(col.$refLinks?.[col.$refLinks.length - 1].definition)) {
const calcElement = resolveCalculatedElement(col, true)
res.push(calcElement)
} else if (col.isJoinRelevant) {
const tableAlias = getQuerySourceName(col)
const name = calculateElementName(col)
const transformedColumn = {
ref: [tableAlias, name],
}
if (col.sort) transformedColumn.sort = col.sort
if (col.nulls) transformedColumn.nulls = col.nulls
res.push(transformedColumn)
} else if (pseudos.elements[col.ref?.[0]]) {
res.push({ ...col })
} else if (col.ref) {
Expand Down Expand Up @@ -1001,8 +991,14 @@ function cqn4sql(originalQuery, model) {
*/
if (inOrderBy && flatColumns.length > 1)
throw new Error(`"${getFullName(leaf)}" can't be used in order by as it expands to multiple fields`)
if (col.nulls) flatColumns[0].nulls = col.nulls
if (col.sort) flatColumns[0].sort = col.sort
flatColumns.forEach(fc => {
if (col.nulls)
fc.nulls = col.nulls
if (col.sort)
fc.sort = col.sort
if (fc.as)
delete fc.as
})
res.push(...flatColumns)
} else {
let transformedColumn
Expand Down
28 changes: 28 additions & 0 deletions db-service/test/cqn4sql/expand.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1050,6 +1050,34 @@ describe('Expands with aggregations are special', () => {
const res = cqn4sql(q, 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,
books { dedication }
} group by books.dedication`

const qx = CQL`SELECT from bookshop.Authors as Authors left join bookshop.Books as books on books.author_ID = Authors.ID {
Authors.ID,
(SELECT from DUMMY { books.dedication_addressee_ID, books.dedication_text, books.dedication_sub_foo, books.dedication_dedication }) as books
} group by books.dedication_addressee_ID, books.dedication_text, books.dedication_sub_foo, books.dedication_dedication`
qx.SELECT.columns[1].SELECT.from = null
const res = cqn4sql(q, model)
expect(JSON.parse(JSON.stringify(res))).to.deep.equal(qx)
})
it('optimized foreign key access', () => {
const q = CQL`SELECT from bookshop.Books {
ID,
Books.author { name, ID }
} group by author.name, author.ID`

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 { author.name, Books.author_ID }) as author
} group by author.name, Books.author_ID`
qx.SELECT.columns[1].SELECT.from = null
const res = cqn4sql(q, model)
expect(JSON.parse(JSON.stringify(res))).to.deep.equal(qx)
})
it('expand path with filter must be an exact match in group by', () => {
const q = CQL`SELECT from bookshop.Books {
Books.ID,
Expand Down
5 changes: 5 additions & 0 deletions test/scenarios/bookshop/read.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ describe('Bookshop - Read', () => {
expect(res.data.value.length).to.be.eq(4) // As there are two books which have the same author
})

test('same as above, with foreign key optimization', async () => {
const res = await GET('/admin/Books?$apply=filter(title%20ne%20%27bar%27)/groupby((author/name, author/ID),aggregate(price with sum as totalAmount))', admin)
expect(res.data.value.length).to.be.eq(4) // As there are two books which have the same author
})

test('Path expression', async () => {
const q = CQL`SELECT title, author.name as author FROM sap.capire.bookshop.Books where author.name LIKE '%a%'`
const res = await cds.run(q)
Expand Down

0 comments on commit 77b7978

Please sign in to comment.