feat: wrap subQuery with parenthesis when it appears as table name (#…

…3496)
knex · Oct 28, 2019 · 0560959 · 0560959
1 parent 1f4d8f2
commit 0560959
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 3 deletions.
diff --git 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
@@ -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/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;
+            }
+          }
+        );
+    });
   });
 };
diff --git 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']))