Skip to content

Commit

Permalink
Support for 'is null' in 'order by' (knex#3667) (knex#4720)
Browse files Browse the repository at this point in the history
  • Loading branch information
@OlivierCavadenti @rustyconover
OlivierCavadenti authored and rustyconover committed Oct 18, 2021
1 parent 700970c commit 285864d
Show file tree
Hide file tree
Showing 6 changed files with 238 additions and 9 deletions.
16 changes: 16 additions & 0 deletions lib/dialects/mssql/query/mssql-querycompiler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const QueryCompiler = require('../../../query/querycompiler');
const compact = require('lodash/compact');
const identity = require('lodash/identity');
const isEmpty = require('lodash/isEmpty');
const Raw = require('../../../raw.js');

const components = [
'columns',
Expand Down Expand Up @@ -226,6 +227,21 @@ class QueryCompiler_MSSQL extends QueryCompiler {
};
}

_formatGroupsItemValue(value, nulls) {
const column = super._formatGroupsItemValue(value);
// MSSQL dont support 'is null' syntax in order by,
// so we override this function and add MSSQL specific syntax.
if (nulls && !(value instanceof Raw)) {
const collNulls = `IIF(${column} is null,`;
if (nulls === 'first') {
return `${collNulls}0,1)`;
} else if (nulls === 'last') {
return `${collNulls}1,0)`;
}
}
return column;
}

standardUpdate() {
const top = this.top();
const withSQL = this.with();
Expand Down
4 changes: 3 additions & 1 deletion lib/query/querybuilder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ class Builder extends EventEmitter {
}

// Adds a `order by` clause to the query.
orderBy(column, direction) {
orderBy(column, direction, nulls = '') {
if (Array.isArray(column)) {
return this._orderByArray(column);
}
Expand All @@ -640,6 +640,7 @@ class Builder extends EventEmitter {
type: 'orderByBasic',
value: column,
direction,
nulls,
});
return this;
}
Expand All @@ -654,6 +655,7 @@ class Builder extends EventEmitter {
type: 'orderByBasic',
value: columnInfo['column'],
direction: columnInfo['order'],
nulls: columnInfo['nulls'],
});
} else if (isString(columnInfo)) {
this._statements.push({
Expand Down
15 changes: 11 additions & 4 deletions lib/query/querycompiler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1226,8 +1226,15 @@ class QueryCompiler {
return vals;
}

_formatGroupsItemValue(value) {
_formatGroupsItemValue(value, nulls) {
const { formatter } = this;
let nullOrder = '';
if (nulls === 'last') {
nullOrder = ' is null';
} else if (nulls === 'first') {
nullOrder = ' is not null';
}

if (value instanceof Raw) {
return unwrapRaw_(
value,
Expand All @@ -1236,8 +1243,8 @@ class QueryCompiler {
this.client,
this.bindingsHolder
);
} else if (value instanceof QueryBuilder) {
return '(' + formatter.columnize(value) + ')';
} else if (value instanceof QueryBuilder || nulls) {
return '(' + formatter.columnize(value) + nullOrder + ')';
} else {
return formatter.columnize(value);
}
Expand All @@ -1248,7 +1255,7 @@ class QueryCompiler {
const items = this.grouped[type];
if (!items) return '';
const sql = items.map((item) => {
const column = this._formatGroupsItemValue(item.value);
const column = this._formatGroupsItemValue(item.value, item.nulls);
const direction =
type === 'order' && item.type !== 'orderByRaw'
? ` ${direction_(
Expand Down
121 changes: 121 additions & 0 deletions test/integration2/query/select/selects.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,127 @@ describe('Selects', function () {
});
});

it('order by with null', async () => {
await knex.schema
.dropTableIfExists('OrderByNullTest')
.createTable('OrderByNullTest', function (table) {
table.increments('id').primary();
table.string('null_col').nullable().defaultTo(null);
});

await knex('OrderByNullTest').insert([
{
null_col: 'test',
},
{
null_col: 'test2',
},
{
null_col: null,
},
{
null_col: null,
},
]);

await knex('OrderByNullTest')
.pluck('id')
.orderBy('null_col', 'asc', 'first')
.testSql(function (tester) {
tester(
'mysql',
'select `id` from `OrderByNullTest` order by (`null_col` is not null) asc',
[],
[3, 4, 1, 2]
);
tester(
'pg',
'select "id" from "OrderByNullTest" order by ("null_col" is not null) asc',
[],
[3, 4, 1, 2]
);
tester(
'pgnative',
'select "id" from "OrderByNullTest" order by ("null_col" is not null) asc',
[],
[3, 4, 1, 2]
);
tester(
'pg-redshift',
'select "id" from "OrderByNullTest" order by ("null_col" is not null) asc',
[],
['3', '4', '1', '2']
);
tester(
'sqlite3',
'select `id` from `OrderByNullTest` order by (`null_col` is not null) asc',
[],
[3, 4, 1, 2]
);
tester(
'oracledb',
'select "id" from "OrderByNullTest" order by ("null_col" is not null) asc',
[],
[3, 4, 1, 2]
);
tester(
'mssql',
'select [id] from [OrderByNullTest] order by IIF([null_col] is null,0,1) asc',
[],
[3, 4, 1, 2]
);
});

await knex('OrderByNullTest')
.pluck('id')
.orderBy('null_col', 'asc', 'last')
.testSql(function (tester) {
tester(
'mysql',
'select `id` from `OrderByNullTest` order by (`null_col` is null) asc',
[],
[1, 2, 3, 4]
);
tester(
'pg',
'select "id" from "OrderByNullTest" order by ("null_col" is null) asc',
[],
[1, 2, 3, 4]
);
tester(
'pgnative',
'select "id" from "OrderByNullTest" order by ("null_col" is null) asc',
[],
[1, 2, 3, 4]
);
tester(
'pg-redshift',
'select "id" from "OrderByNullTest" order by ("null_col" is null) asc',
[],
['1', '2', '3', '4']
);
tester(
'sqlite3',
'select `id` from `OrderByNullTest` order by (`null_col` is null) asc',
[],
[1, 2, 3, 4]
);
tester(
'oracledb',
'select "id" from "OrderByNullTest" order by ("null_col" is null) asc',
[],
[1, 2, 3, 4]
);
tester(
'mssql',
'select [id] from [OrderByNullTest] order by IIF([null_col] is null,1,0) asc',
[],
[1, 2, 3, 4]
);
});
await knex.schema.dropTable('OrderByNullTest');
});

describe('simple "where" cases', function () {
it('allows key, value', function () {
return knex('accounts')
Expand Down
78 changes: 78 additions & 0 deletions test/unit/query/builder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5625,6 +5625,84 @@ describe('QueryBuilder', () => {
);
});

it('order by, null first', () => {
testsql(qb().from('users').orderBy('foo', 'desc', 'first'), {
mysql: {
sql: 'select * from `users` order by (`foo` is not null) desc',
},
mssql: {
sql: 'select * from [users] order by IIF([foo] is null,0,1) desc',
},
pg: {
sql: 'select * from "users" order by ("foo" is not null) desc',
},
'pg-redshift': {
sql: 'select * from "users" order by ("foo" is not null) desc',
},
});
});

it('order by, null first, array notation', () => {
testsql(
qb()
.from('users')
.orderBy([{ column: 'foo', order: 'desc', nulls: 'first' }]),
{
mysql: {
sql: 'select * from `users` order by (`foo` is not null) desc',
},
mssql: {
sql: 'select * from [users] order by IIF([foo] is null,0,1) desc',
},
pg: {
sql: 'select * from "users" order by ("foo" is not null) desc',
},
'pg-redshift': {
sql: 'select * from "users" order by ("foo" is not null) desc',
},
}
);
});

it('order by, null last', () => {
testsql(qb().from('users').orderBy('foo', 'desc', 'last'), {
mysql: {
sql: 'select * from `users` order by (`foo` is null) desc',
},
mssql: {
sql: 'select * from [users] order by IIF([foo] is null,1,0) desc',
},
pg: {
sql: 'select * from "users" order by ("foo" is null) desc',
},
'pg-redshift': {
sql: 'select * from "users" order by ("foo" is null) desc',
},
});
});

it('order by, null last, array notation', () => {
testsql(
qb()
.from('users')
.orderBy([{ column: 'foo', order: 'desc', nulls: 'last' }]),
{
mysql: {
sql: 'select * from `users` order by (`foo` is null) desc',
},
mssql: {
sql: 'select * from [users] order by IIF([foo] is null,1,0) desc',
},
pg: {
sql: 'select * from "users" order by ("foo" is null) desc',
},
'pg-redshift': {
sql: 'select * from "users" order by ("foo" is null) desc',
},
}
);
});

it('update method with joins mysql', () => {
testsql(
qb()
Expand Down
13 changes: 9 additions & 4 deletions types/index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1483,7 +1483,10 @@ export declare namespace Knex {
TAlias extends string,
TKey extends keyof ResolveTableType<TRecord>,
TResult2 = AggregationQueryResult<TResult, {[x in TAlias]: number}>
>(alias: TAlias, orderBy: TKey | TKey[] | { columnName: TKey, order?: 'asc' | 'desc' }, partitionBy?: TKey | TKey[] | { columnName: TKey, order?: 'asc' | 'desc' }): QueryBuilder<
>(alias: TAlias,
orderBy: TKey | TKey[] | { columnName: TKey, order?: 'asc' | 'desc', nulls?: 'first' | 'last' },
partitionBy?: TKey | TKey[] | { columnName: TKey, order?: 'asc' | 'desc' }):
QueryBuilder<
TRecord,
TResult2
>;
Expand All @@ -1494,23 +1497,25 @@ export declare namespace Knex {
ColumnNameQueryBuilder<TRecord, TResult> {}

interface OrderBy<TRecord = any, TResult = unknown[]> {
(columnName: keyof TRecord | QueryBuilder, order?: 'asc' | 'desc'): QueryBuilder<
(columnName: keyof TRecord | QueryBuilder, order?: 'asc' | 'desc', nulls?: 'first' | 'last'): QueryBuilder<
TRecord,
TResult
>;
(columnName: string | QueryBuilder, order?: string): QueryBuilder<TRecord, TResult>;
(columnName: string | QueryBuilder, order?: string, nulls?: string): QueryBuilder<TRecord, TResult>;
(
columnDefs: Array<
keyof TRecord | Readonly<{
column: keyof TRecord | QueryBuilder;
order?: 'asc' | 'desc'
order?: 'asc' | 'desc',
nulls?: 'first' | 'last'
}>
>
): QueryBuilder<TRecord, TResult>;
(
columnDefs: Array<string | Readonly<{
column: string | QueryBuilder;
order?: string;
nulls?: string;
}>>
): QueryBuilder<TRecord, TResult>;
}
Expand Down

0 comments on commit 285864d

Please sign in to comment.