Skip to content

Commit

Permalink
fix: consider nulls first | last on orderBy (#1064)
Browse files Browse the repository at this point in the history
fix #1062
  • Loading branch information
patricebender authored Mar 10, 2025
1 parent fcde2f2 commit c6bed60
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 21 deletions.
13 changes: 7 additions & 6 deletions db-service/lib/cqn2sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,14 +421,15 @@ class CQN2SQLRenderer {
* @returns {string[] | string} SQL
*/
orderBy(orderBy, localized) {
return orderBy.map(
localized
? c =>
this.expr(c) +
return orderBy.map(c => {
const o = localized
? this.expr(c) +
(c.element?.[this.class._localized] ? ' COLLATE NOCASE' : '') +
(c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? ' DESC' : ' ASC')
: c => this.expr(c) + (c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? ' DESC' : ' ASC'),
)
: this.expr(c) + (c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? ' DESC' : ' ASC')
if (c.nulls) return o + ' NULLS ' + (c.nulls.toLowerCase() === 'first' ? 'FIRST' : 'LAST')
return o
})
}

/**
Expand Down
16 changes: 8 additions & 8 deletions hana/lib/HANAService.js
Original file line number Diff line number Diff line change
Expand Up @@ -904,17 +904,17 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction})) AS NEW LE
}

orderBy(orderBy, localized) {
return orderBy.map(
localized
? c =>
this.expr(c) +
return orderBy.map(c => {
const o = localized
? this.expr(c) +
(c.element?.[this.class._localized]
? ` COLLATE ${collations[this.context.locale] || collations[this.context.locale.split('_')[0]] || collations['']
}`
? ` COLLATE ${collations[this.context.locale] || collations[this.context.locale.split('_')[0]] || collations['']}`
: '') +
(c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? ' DESC' : ' ASC')
: c => this.expr(c) + (c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? ' DESC' : ' ASC'),
)
: this.expr(c) + (c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? ' DESC' : ' ASC')
if (c.nulls) return o + ' NULLS ' + (c.nulls.toLowerCase() === 'first' ? 'FIRST' : 'LAST')
return o
})
}

limit({ rows, offset }) {
Expand Down
15 changes: 8 additions & 7 deletions postgres/lib/PostgresService.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,14 +344,15 @@ GROUP BY k

static CQN2SQL = class CQN2Postgres extends SQLService.CQN2SQL {
_orderBy(orderBy, localized, locale) {
return orderBy.map(
localized
? c =>
this.expr(c) +
return orderBy.map(c => {
const nulls = c.nulls || (c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? 'LAST' : 'FIRST')
const o = localized
? this.expr(c) +
(c.element?.[this.class._localized] ? ` COLLATE "${locale}"` : '') +
(c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? ' DESC NULLS LAST' : ' ASC NULLS FIRST')
: c => this.expr(c) + (c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? ' DESC NULLS LAST' : ' ASC NULLS FIRST'),
)
(c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? ' DESC' : ' ASC')
: this.expr(c) + (c.sort?.toLowerCase() === 'desc' || c.sort === -1 ? ' DESC' : ' ASC')
return o + ' NULLS ' + (nulls.toLowerCase() === 'first' ? 'FIRST' : 'LAST')
})
}

orderBy(orderBy) {
Expand Down
9 changes: 9 additions & 0 deletions test/scenarios/bookshop/orderBy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,13 @@ describe('Bookshop - Order By', () => {
expect(res.length).to.be.eq(1)
expect(res[0].author).to.eq('Charlotte Brontë')
})

test('nulls first | last', async () => {
const { Authors } = cds.entities('sap.capire.bookshop')
await INSERT.into(Authors).entries({ ID: 42, name: 'Brandon Sanderson' }) // dateOfDeath => null
const nullsFirst = await cds.ql`SELECT from ${Authors} { name } order by dateOfDeath asc nulls first`
expect(nullsFirst[0].name).to.eq('Brandon Sanderson')
const nullsLast = await cds.ql`SELECT from ${Authors} { name } order by dateOfDeath asc nulls last`
expect(nullsLast.at(-1).name).to.eq('Brandon Sanderson')
});
})

0 comments on commit c6bed60

Please sign in to comment.