Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wildcard expand with aggregation #923

Merged
merged 6 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this also representing an issue?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, one that I found... will tackle this separately

// 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
Loading