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: Force sql_simple_queries to quote in outer select #954

Merged
merged 4 commits into from
Dec 18, 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
8 changes: 6 additions & 2 deletions hana/lib/HANAService.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ class HANAService extends SQLService {
throw new Error('CQN query using joins must specify the selected columns.')
}

let { limit, one, from, orderBy, expand, columns = ['*'], localized, count, parent } = q.SELECT
let { limit, one, from, orderBy, having, expand, columns = ['*'], localized, count, parent } = q.SELECT

// When one of these is defined wrap the query in a sub query
if (expand || (parent && (limit || one || orderBy))) {
Expand Down Expand Up @@ -371,6 +371,7 @@ class HANAService extends SQLService {
columns.push({ ref: [parent.as, '_path_'], as: '_parent_path_' })
}

let orderByHasOutputColumnRef = false
if (orderBy) {
// Ensure that all columns used in the orderBy clause are exposed
orderBy = orderBy.map((c, i) => {
Expand All @@ -388,6 +389,7 @@ class HANAService extends SQLService {
}
return { __proto__: c, ref: [this.column_name(match || c)], sort: c.sort }
}
orderByHasOutputColumnRef = true
return c
})
}
Expand Down Expand Up @@ -426,7 +428,9 @@ class HANAService extends SQLService {
q.as = q.SELECT.from.as
}

if (rowNumberRequired || q.SELECT.columns.length !== aliasedOutputColumns.length) {
const outputAliasSimpleQueriesRequired = cds.env.features.sql_simple_queries
&& (orderByHasOutputColumnRef || having)
if (outputAliasSimpleQueriesRequired || rowNumberRequired || q.SELECT.columns.length !== aliasedOutputColumns.length) {
q = cds.ql.SELECT(aliasedOutputColumns).from(q)
q.as = q.SELECT.from.as
Object.defineProperty(q, 'elements', { value: elements })
Expand Down
25 changes: 18 additions & 7 deletions test/scenarios/bookshop/read.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ describe('Bookshop - Read', () => {
expect(res.data.value.length).to.be.eq(4) // As there are two books which have the same author
expect(
res.data.value.every(
item =>
'author' in item &&
'ID' in item.author && // foreign key is renamed to element name in target
!('author_ID' in item.author),
item =>
'author' in item &&
'ID' in item.author && // foreign key is renamed to element name in target
!('author_ID' in item.author),
),
).to.be.true
})
Expand Down Expand Up @@ -134,6 +134,17 @@ describe('Bookshop - Read', () => {
expect(res.length).to.be.eq(2)
})

test('order by computed result column', async () => {
const { Authors } = cds.entities('sap.capire.bookshop')
const res = await SELECT
.columns`ID,sum(books_price) as price :Decimal`
.from(CQL`SELECT ID,books.price from ${Authors}`)
.groupBy`ID`
.orderBy`price desc`
expect(res.length).to.be.eq(4)
expect(res[0].price).to.be.eq('150')
})

test('reuse already executed select as subselect', async () => {
let s = SELECT.columns('ID').from('sap.capire.bookshop.Books')
let res = await s
Expand Down Expand Up @@ -370,9 +381,9 @@ describe('Bookshop - Read', () => {
})

it('allows filtering with between operator', async () => {
const query = SELECT.from('sap.capire.bookshop.Books', ['ID', 'stock']).where ({ stock: { between: 0, and: 100 } })
const query = SELECT.from('sap.capire.bookshop.Books', ['ID', 'stock']).where({ stock: { between: 0, and: 100 } })

return expect((await query).every(row => row.stock >=0 && row.stock <=100)).to.be.true
return expect((await query).every(row => row.stock >= 0 && row.stock <= 100)).to.be.true
})

it('allows various mechanisms for expressing "not in"', async () => {
Expand All @@ -382,7 +393,7 @@ describe('Bookshop - Read', () => {
SELECT.from('sap.capire.bookshop.Books', ['ID']).where('ID not in', [201, 251]).orderBy('ID'),
])

for (const row of results) expect(row).to.deep.eq([{ID: 207},{ID: 252},{ID: 271}])
for (const row of results) expect(row).to.deep.eq([{ ID: 207 }, { ID: 252 }, { ID: 271 }])
})

it('select all authors which have written books that have genre.name = null', async () => {
Expand Down
Loading