Skip to content

Commit

Permalink
fix: wildcard expand with aggregation (#923)
Browse files Browse the repository at this point in the history
for the case where we `group by` expanded columns, we must ignore the
`expand` from the columns
  • Loading branch information
patricebender authored Dec 5, 2024
1 parent 2c72c87 commit bbe7be0
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 7 deletions.
17 changes: 10 additions & 7 deletions db-service/lib/cqn4sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -873,7 +874,13 @@ function cqn4sql(originalQuery, model) {

// to be attached to dummy query
const elements = {}
const wildcardIndex = column.expand.findIndex(e => e === '*')
if (wildcardIndex !== -1) {
// 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
const fullRef = [...baseRef, ...expand.ref]

if (expand.expand) {
Expand Down Expand Up @@ -1404,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
Expand Down Expand Up @@ -2204,10 +2210,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 {
Expand Down
29 changes: 29 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,35 @@ 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('wildcard expand vanishes for aggregations', () => {
const q = CQL`SELECT from bookshop.TestPublisher {
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
} group by TestPublisher.ID, TestPublisher.publisher_structuredKey_ID, publisher.title`
// 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,
Expand Down

0 comments on commit bbe7be0

Please sign in to comment.