Skip to content

Commit

Permalink
fix: wrong odata count in filter with groupby (#352)
Browse files Browse the repository at this point in the history
Encase the groupby within an additional SELECT statement to ensure
accurate row counting.

---------

Co-authored-by: I543501 <lars.lutz@sap.com>
Co-authored-by: Johannes Vogel <31311694+johannes-vogel@users.noreply.github.com>
  • Loading branch information
3 people authored Nov 29, 2023
1 parent 7d8521a commit 70690a1
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 8 deletions.
15 changes: 7 additions & 8 deletions db-service/lib/SQLService.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,16 +230,15 @@ class SQLService extends DatabaseService {
if (max === undefined || (n < max && (n || !offset))) return n + offset
}
// REVISIT: made uppercase count because of HANA reserved word quoting
const cq = cds.ql.clone(query, {
columns: [{ func: 'count', as: 'COUNT' }],
const cq = SELECT.one([{ func: 'count', as: 'COUNT' }]).from(
cds.ql.clone(query, {
localized: false,
expand: false,
limit: 0,
orderBy: 0,
})
const { sql, values } = this.cqn2sql(cq)
const ps = await this.prepare(sql)
const { count, COUNT } = await ps.get(values)
limit: undefined,
orderBy: undefined,
}),
)
const { count, COUNT } = await this.onSELECT({ query: cq })
return count ?? COUNT
}

Expand Down
9 changes: 9 additions & 0 deletions test/scenarios/bookshop/read.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ describe('Bookshop - Read', () => {
expect(res.data['@odata.count']).to.be.eq(5)
})

test('Books $count with $top=1 and groupby', async () => {
const res = await GET(
`/browse/ListOfBooks?$apply=groupby((ID),aggregate(ID with countdistinct as countBookings))&$count=true&$top=1`,
)
expect(res.status).to.be.eq(200)
expect(res.data.value.length).to.be.eq(1)
expect(res.data['@odata.count']).to.be.eq(5)
})

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
1 change: 1 addition & 0 deletions test/scenarios/sflight/read.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('SFlight - Read', () => {
// REVISIT: works in sflight not in tests
// 'Bookings?$apply=concat(groupby((BookingID,ConnectionID,CurrencyCode_code,FlightDate,ID,TravelID,airline,status))/aggregate($count%20as%20UI5__leaves),aggregate(FlightPrice,CurrencyCode_code),groupby((airline,airlineName),aggregate(FlightPrice,CurrencyCode_code))/concat(aggregate($count%20as%20UI5__count),top(53)))',
'Bookings?$apply=groupby((airline,airlineName),aggregate(FlightPrice with average as avgPrice,FlightPrice with max as maxPrice,FlightPrice with min as minPrice))&$skip=0&$top=1',
`Bookings?$apply=filter(airline eq 'EA' and status eq 'B')/groupby((ID),aggregate(FlightPrice,CurrencyCode_code))&$count=true&$skip=0&$top=1`
]

test.each(analyticsPaths)('/analytics/%s', async p => {
Expand Down

0 comments on commit 70690a1

Please sign in to comment.