Skip to content

Commit

Permalink
fix null orderby
Browse files Browse the repository at this point in the history
  • Loading branch information
@OlivierCavadenti
OlivierCavadenti committed Dec 14, 2021
1 parent f38e57c commit ccd281d
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 42 deletions.
8 changes: 4 additions & 4 deletions lib/dialects/mssql/query/mssql-querycompiler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,19 +201,19 @@ class QueryCompiler_MSSQL extends QueryCompiler {
};
}

_formatGroupsItemValue(value, nulls) {
_formatGroupsItemValue(value, nulls, direction) {
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)`;
return `${collNulls}0,1)${direction}`;
} else if (nulls === 'last') {
return `${collNulls}1,0)`;
return `${collNulls}1,0)${direction}`;
}
}
return column;
return column + direction;
}

standardUpdate() {
Expand Down
9 changes: 9 additions & 0 deletions lib/dialects/oracledb/query/oracledb-querycompiler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const Oracle_Compiler = require('../../oracle/query/oracle-querycompiler');
const ReturningHelper = require('../utils').ReturningHelper;
const BlobHelper = require('../utils').BlobHelper;
const { isString } = require('../../../util/is');
const Raw = require('../../../raw.js');

class Oracledb_Compiler extends Oracle_Compiler {
// Compiles an "insert" query, allowing for multiple
Expand Down Expand Up @@ -312,6 +313,14 @@ class Oracledb_Compiler extends Oracle_Compiler {
return result;
}

_formatGroupsItemValue(value, nulls, direction) {
const column = super._formatGroupsItemValue(value);
if (nulls && !(value instanceof Raw)) {
return `${column}${direction} nulls ${nulls}`;
}
return column;
}

update() {
const self = this;
const sql = {};
Expand Down
13 changes: 7 additions & 6 deletions lib/query/querycompiler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1265,7 +1265,7 @@ class QueryCompiler {
return vals;
}

_formatGroupsItemValue(value, nulls) {
_formatGroupsItemValue(value, nulls, direction) {
const { formatter } = this;
let nullOrder = '';
if (nulls === 'last') {
Expand All @@ -1274,27 +1274,28 @@ class QueryCompiler {
nullOrder = ' is not null';
}

let groupOrder;
if (value instanceof Raw) {
return unwrapRaw_(
groupOrder = unwrapRaw_(
value,
undefined,
this.builder,
this.client,
this.bindingsHolder
);
} else if (value instanceof QueryBuilder || nulls) {
return '(' + formatter.columnize(value) + nullOrder + ')';
groupOrder = '(' + formatter.columnize(value) + nullOrder + ')';
} else {
return formatter.columnize(value);
groupOrder = formatter.columnize(value);
}
return groupOrder + (direction ? direction : '');
}

// Compiles the `order by` statements.
_groupsOrders(type) {
const items = this.grouped[type];
if (!items) return '';
const sql = items.map((item) => {
const column = this._formatGroupsItemValue(item.value, item.nulls);
const direction =
type === 'order' && item.type !== 'orderByRaw'
? ` ${direction_(
Expand All @@ -1304,7 +1305,7 @@ class QueryCompiler {
this.bindingsHolder
)}`
: '';
return column + direction;
return this._formatGroupsItemValue(item.value, item.nulls, direction);
});
return sql.length ? type + ' by ' + sql.join(', ') : '';
}
Expand Down
76 changes: 44 additions & 32 deletions test/integration2/query/select/selects.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,116 +565,128 @@ describe('Selects', function () {
.createTable('OrderByNullTest', function (table) {
table.increments('id').primary();
table.string('null_col').nullable().defaultTo(null);
// string col to have some order of records with nulls
table.string('string_col');
});

await knex('OrderByNullTest').insert([
{
null_col: 'test',
string_col: 'a',
},
{
null_col: 'test2',
null_col: null,
string_col: 'b',
},
{
null_col: null,
null_col: 'test2',
string_col: 'c',
},
{
null_col: null,
string_col: 'd',
},
]);

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

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

0 comments on commit ccd281d

Please sign in to comment.