From ef124fa2b7ce1ffb8cf2473c017489bd1f2590dd Mon Sep 17 00:00:00 2001 From: edvardchen <> Date: Fri, 25 Oct 2019 00:39:09 +0800 Subject: [PATCH 1/2] feat: wrap subQuery with parenthesis when it appears as table name Fixes #3485 --- lib/formatter.js | 4 ++-- lib/query/compiler.js | 5 ++++- test/unit/query/builder.js | 31 +++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/lib/formatter.js b/lib/formatter.js index fc46ef11e1..d7831727db 100644 --- a/lib/formatter.js +++ b/lib/formatter.js @@ -171,8 +171,8 @@ class Formatter { // Puts the appropriate wrapper around a value depending on the database // engine, unless it's a knex.raw value, in which case it's left alone. - wrap(value) { - const raw = this.unwrapRaw(value); + wrap(value, isParameter) { + const raw = this.unwrapRaw(value, isParameter); if (raw) return raw; switch (typeof value) { case 'function': diff --git a/lib/query/compiler.js b/lib/query/compiler.js index 89170cb1ee..535d021709 100644 --- a/lib/query/compiler.js +++ b/lib/query/compiler.js @@ -865,7 +865,10 @@ Object.defineProperty(QueryCompiler.prototype, 'tableName', { if (tableName && schemaName) tableName = `${schemaName}.${tableName}`; - this._tableName = tableName ? this.formatter.wrap(tableName) : ''; + this._tableName = tableName + ? // Wrap subQuery with parenthesis, #3485 + this.formatter.wrap(tableName, tableName instanceof QueryBuilder) + : ''; } return this._tableName; }, diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js index 2f91e1bcfc..742fadd9e5 100644 --- a/test/unit/query/builder.js +++ b/test/unit/query/builder.js @@ -8179,6 +8179,37 @@ describe('QueryBuilder', () => { ); }); + it('should always wrap subquery with parenthesis', () => { + const subquery = qb().select(raw('?', ['inner raw select']), 'bar'); + testsql( + qb() + .select(raw('?', ['outer raw select'])) + .from(subquery), + { + mysql: { + sql: 'select ? from (select ?, `bar`)', + bindings: ['outer raw select', 'inner raw select'], + }, + mssql: { + sql: 'select ? from (select ?, [bar])', + bindings: ['outer raw select', 'inner raw select'], + }, + oracledb: { + sql: 'select ? from (select ?, "bar")', + bindings: ['outer raw select', 'inner raw select'], + }, + pg: { + sql: 'select ? from (select ?, "bar")', + bindings: ['outer raw select', 'inner raw select'], + }, + 'pg-redshift': { + sql: 'select ? from (select ?, "bar")', + bindings: ['outer raw select', 'inner raw select'], + }, + } + ); + }); + it('correctly orders parameters when selecting from subqueries, #704', () => { const subquery = qb() .select(raw('? as f', ['inner raw select'])) From 1e996f7fc4c406559b466ad692ef8a442f638cfc Mon Sep 17 00:00:00 2001 From: edvardchen <> Date: Mon, 28 Oct 2019 22:24:35 +0800 Subject: [PATCH 2/2] test: add integration tests for #3485 --- test/integration/builder/selects.js | 33 +++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js index 6c89e47023..1cc117ee27 100644 --- a/test/integration/builder/selects.js +++ b/test/integration/builder/selects.js @@ -1380,5 +1380,38 @@ module.exports = function(knex) { } } }); + + it('select from subquery', async function() { + const subquery = knex.from('accounts').whereBetween('id', [3, 5]); + return knex + .pluck('id') + .orderBy('id') + .from(subquery) + .then( + (rows) => { + expect(rows).to.deep.equal([3, 4, 5]); + expect(knex.client.driverName).to.oneOf(['sqlite3', 'oracledb']); + }, + (e) => { + switch (knex.client.driverName) { + case 'mysql': + case 'mysql2': + expect(e.errno).to.equal(1248); + break; + case 'pg': + expect(e.message).to.contain('must have an alias'); + break; + + case 'mssql': + expect(e.message).to.contain( + "Incorrect syntax near the keyword 'order'" + ); + break; + default: + throw e; + } + } + ); + }); }); };