Skip to content

Commit

Permalink
fix: Force sql_simple_queries to quote in outer select (#954)
Browse files Browse the repository at this point in the history
Co-authored-by: Johannes Vogel <[email protected]>
  • Loading branch information
BobdenOs and johannes-vogel authored Dec 18, 2024
1 parent 3f45dc2 commit 2071dc8
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 9 deletions.
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

0 comments on commit 2071dc8

Please sign in to comment.