Skip to content

Commit

Permalink
Use columnize instead of wrap in using(). (#2713)
Browse files Browse the repository at this point in the history
* Use columnize instead of wrap in using().

This is an attempt to fix #2136. Also added an integration test, couldn't find any existing.

* Exclude MSSQL from test as it does not support .using

* Change test to not use subquery

* Change test
  • Loading branch information
Simon Lidén authored and elhigu committed Aug 29, 2018
1 parent 89d2b3a commit 1616b2a
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/query/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ assign(QueryCompiler.prototype, {
},

onUsing(clause) {
return this.formatter.wrap(clause.column);
return '(' + this.formatter.columnize(clause.column) + ')';
},

// Where Clause
Expand Down
50 changes: 50 additions & 0 deletions test/integration/builder/joins.js
Original file line number Diff line number Diff line change
Expand Up @@ -1889,5 +1889,55 @@ module.exports = function(knex) {
);
});
});

if (knex.client.driverName !== 'mssql') {
it('Can use .using()', () => {
let joinName = 'accounts_join_test';

return knex.schema
.dropTableIfExists(joinName)
.then(() =>
knex.schema.createTable(joinName, (table) => {
table.bigint('id');
table.string('email');
table.integer('testcolumn');
})
)
.then(() =>
knex(joinName).insert([
{
id: 3,
email: 'test3@example.com',
testcolumn: 50,
},
{
id: 3,
email: 'random@email.com',
testcolumn: 70,
},
])
)
.then(() =>
knex('accounts').join(joinName, (builder) =>
builder.using(['id', 'email'])
)
)
.then((rows) => {
expect(rows.length).to.equal(1);
expect(rows[0].testcolumn).to.equal(50);

return knex('accounts')
.join(joinName, (builder) => builder.using(['id']))
.orderBy('testcolumn');
})
.then((rows) => {
expect(rows.length).to.equal(2);
expect(rows[0].testcolumn).to.equal(50);
expect(rows[1].testcolumn).to.equal(70);

return true;
});
});
}
});
};
36 changes: 32 additions & 4 deletions test/unit/query/builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -6721,17 +6721,45 @@ describe('QueryBuilder', function() {
}),
{
mysql: {
sql: 'select * from `accounts` inner join `table1` using `id`',
sql: 'select * from `accounts` inner join `table1` using (`id`)',
},
mssql: {
//sql: 'select * from [accounts] inner join [table1] on [accounts].[id] = [table1].[id]'
sql: 'select * from [accounts] inner join [table1] using [id]',
sql: 'select * from [accounts] inner join [table1] using ([id])',
},
pg: {
sql: 'select * from "accounts" inner join "table1" using "id"',
sql: 'select * from "accounts" inner join "table1" using ("id")',
},
'pg-redshift': {
sql: 'select * from "accounts" inner join "table1" using "id"',
sql: 'select * from "accounts" inner join "table1" using ("id")',
},
}
);

testsql(
qb()
.select('*')
.from('accounts')
.innerJoin('table1', function() {
this.using(['id', 'test']);
}),
{
mysql: {
sql:
'select * from `accounts` inner join `table1` using (`id`, `test`)',
},
mssql: {
//sql: 'select * from [accounts] inner join [table1] on [accounts].[id] = [table1].[id]'
sql:
'select * from [accounts] inner join [table1] using ([id], [test])',
},
pg: {
sql:
'select * from "accounts" inner join "table1" using ("id", "test")',
},
'pg-redshift': {
sql:
'select * from "accounts" inner join "table1" using ("id", "test")',
},
}
);
Expand Down

0 comments on commit 1616b2a

Please sign in to comment.