Skip to content

Commit

Permalink
fix all union tests and another orderBy null fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@OlivierCavadenti
OlivierCavadenti committed Dec 14, 2021
1 parent a259742 commit f5a8156
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 40 deletions.
22 changes: 17 additions & 5 deletions lib/dialects/oracledb/query/oracledb-querycompiler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ const ReturningHelper = require('../utils').ReturningHelper;
const BlobHelper = require('../utils').BlobHelper;
const { isString } = require('../../../util/is');
const Raw = require('../../../raw.js');
const {
direction: direction_,
} = require('../../../formatter/wrappingFormatter');

class Oracledb_Compiler extends Oracle_Compiler {
// Compiles an "insert" query, allowing for multiple
Expand Down Expand Up @@ -313,12 +316,21 @@ 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}`;
_groupOrder(item, type) {
const column = super._formatGroupsItemValue(item.value);
const direction =
type === 'order' && item.type !== 'orderByRaw'
? ` ${direction_(
item.direction,
this.builder,
this.client,
this.bindingsHolder
)}`
: '';
if (item.nulls && !(item.value instanceof Raw)) {
return `${column}${direction ? direction : ''} nulls ${item.nulls}`;
}
return column;
return column + direction;
}

update() {
Expand Down
26 changes: 15 additions & 11 deletions lib/query/querycompiler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1291,22 +1291,26 @@ class QueryCompiler {
return groupOrder;
}

_groupOrder(item, type) {
const column = this._formatGroupsItemValue(item.value, item.nulls);
const direction =
type === 'order' && item.type !== 'orderByRaw'
? ` ${direction_(
item.direction,
this.builder,
this.client,
this.bindingsHolder
)}`
: '';
return column + 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_(
item.direction,
this.builder,
this.client,
this.bindingsHolder
)}`
: '';
return column + direction;
return this._groupOrder(item, type);
});
return sql.length ? type + ' by ' + sql.join(', ') : '';
}
Expand Down
4 changes: 2 additions & 2 deletions test/integration2/query/select/selects.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ describe('Selects', function () {
);
tester(
'oracledb',
'select "id" from "OrderByNullTest" order by "null_col" asc nulls first, "string_col"',
'select "id" from "OrderByNullTest" order by "null_col" asc nulls first, "string_col" asc',
[],
[2, 4, 1, 3]
);
Expand Down Expand Up @@ -678,7 +678,7 @@ describe('Selects', function () {
);
tester(
'oracledb',
'select "id" from "OrderByNullTest" order by "null_col" asc nulls last, "string_col"',
'select "id" from "OrderByNullTest" order by "null_col" asc nulls last, "string_col" asc',
[],
[1, 3, 2, 4]
);
Expand Down
72 changes: 50 additions & 22 deletions test/integration2/query/select/unions.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('unions', function () {
getAllDbs().forEach((db) => {
describe(db, () => {
let knex;
const unionCols = ['last_name', 'phone'];

before(async () => {
knex = getKnexForDb(db);
Expand All @@ -37,10 +38,17 @@ describe('unions', function () {
await createTestTableTwo(knex);
await createDefaultTable(knex);
await createDefaultTable(knex, true);
await knex.schema.dropTableIfExists('union_raw_test');
await knex.schema.createTable('union_raw_test', (table) => {
table.bigIncrements('id');
table.string('last_name');
table.string('phone');
});
});

beforeEach(async () => {
await knex('accounts').truncate();
await knex('union_raw_test').truncate();
await insertAccounts(knex);
});

Expand All @@ -50,87 +58,107 @@ describe('unions', function () {

it('handles unions with a callback', function () {
return knex('accounts')
.select('*')
.select(unionCols)
.where('id', '=', 1)
.union(function () {
this.select('*').from('accounts').where('id', 2);
this.select(unionCols).from('accounts').where('id', 2);
});
});

it('handles unions with an array of callbacks', function () {
return knex('accounts')
.select('*')
.select(unionCols)
.where('id', '=', 1)
.union([
function () {
this.select('*').from('accounts').where('id', 2);
this.select(unionCols).from('accounts').where('id', 2);
},
function () {
this.select('*').from('accounts').where('id', 3);
this.select(unionCols).from('accounts').where('id', 3);
},
]);
});

it('handles unions with a list of callbacks', function () {
return knex('accounts')
.select('*')
.select(unionCols)
.where('id', '=', 1)
.union(
function () {
this.select('*').from('accounts').where('id', 2);
this.select(unionCols).from('accounts').where('id', 2);
},
function () {
this.select('*').from('accounts').where('id', 3);
this.select(unionCols).from('accounts').where('id', 3);
}
);
});

it('handles unions with an array of builders', function () {
return knex('accounts')
.select('*')
.select(unionCols)
.where('id', '=', 1)
.union([
knex.select('*').from('accounts').where('id', 2),
knex.select('*').from('accounts').where('id', 3),
knex.select(unionCols).from('accounts').where('id', 2),
knex.select(unionCols).from('accounts').where('id', 3),
]);
});

it('handles unions with a list of builders', function () {
return knex('accounts')
.select('*')
.select(unionCols)
.where('id', '=', 1)
.union(
knex.select('*').from('accounts').where('id', 2),
knex.select('*').from('accounts').where('id', 3)
knex.select(unionCols).from('accounts').where('id', 2),
knex.select(unionCols).from('accounts').where('id', 3)
);
});

it('handles unions with a raw query', function () {
return knex('accounts')
return knex('union_raw_test')
.select('*')
.where('id', '=', 1)
.union(
knex.raw('select * from ?? where ?? = ?', ['accounts', 'id', 2])
knex.raw('select * from ?? where ?? = ?', [
'union_raw_test',
'id',
2,
])
);
});

it('handles unions with an array raw queries', function () {
return knex('accounts')
return knex('union_raw_test')
.select('*')
.where('id', '=', 1)
.union([
knex.raw('select * from ?? where ?? = ?', ['accounts', 'id', 2]),
knex.raw('select * from ?? where ?? = ?', ['accounts', 'id', 3]),
knex.raw('select * from ?? where ?? = ?', [
'union_raw_test',
'id',
2,
]),
knex.raw('select * from ?? where ?? = ?', [
'union_raw_test',
'id',
3,
]),
]);
});

it('handles unions with a list of raw queries', function () {
return knex('accounts')
return knex('union_raw_test')
.select('*')
.where('id', '=', 1)
.union(
knex.raw('select * from ?? where ?? = ?', ['accounts', 'id', 2]),
knex.raw('select * from ?? where ?? = ?', ['accounts', 'id', 3])
knex.raw('select * from ?? where ?? = ?', [
'union_raw_test',
'id',
2,
]),
knex.raw('select * from ?? where ?? = ?', [
'union_raw_test',
'id',
3,
])
);
});

Expand Down

0 comments on commit f5a8156

Please sign in to comment.