Fix Analytic orderBy and partitionBy to follow the SQL documentation (#…

…4602)
knex · Aug 30, 2021 · 4c79ac1 · 4c79ac1
1 parent 9573fd0
commit 4c79ac1
Show file tree

Hide file tree

Showing 5 changed files with 204 additions and 175 deletions.
diff --git a/lib/query/analytic.js b/lib/query/analytic.js
@@ -18,7 +18,7 @@ class Analytic {
     this.grouping = 'columns';
   }
 
-  partitionBy(column) {
+  partitionBy(column, direction) {
     assert(
       Array.isArray(column) || typeof column === 'string',
       `The argument to an analytic partitionBy function must be either a string
@@ -28,12 +28,12 @@ class Analytic {
     if (Array.isArray(column)) {
       this.partitions = this.partitions.concat(column);
     } else {
-      this.partitions.push(column);
+      this.partitions.push({ column: column, order: direction });
     }
     return this;
   }
 
-  orderBy(column) {
+  orderBy(column, direction) {
     assert(
       Array.isArray(column) || typeof column === 'string',
       `The argument to an analytic orderBy function must be either a string
@@ -43,7 +43,7 @@ class Analytic {
     if (Array.isArray(column)) {
       this.order = this.order.concat(column);
     } else {
-      this.order.push(column);
+      this.order.push({ column: column, order: direction });
     }
     return this;
   }

diff --git a/lib/query/querybuilder.js b/lib/query/querybuilder.js
@@ -1247,15 +1247,18 @@ class Builder extends EventEmitter {
       typeof second === 'function' ||
         second.isRawInstance ||
         Array.isArray(second) ||
-        typeof second === 'string',
+        typeof second === 'string' ||
+        typeof second === 'object',
       `The second argument to an analytic function must be either a function, a raw,
-       an array of string or a single string.`
+       an array of string or object, an object or a single string.`
     );
 
     if (third) {
       assert(
-        Array.isArray(third) || typeof third === 'string',
-        'The third argument to an analytic function must be either a string or an array of string.'
+        Array.isArray(third) ||
+          typeof third === 'string' ||
+          typeof third === 'object',
+        'The third argument to an analytic function must be either a string, an array of string or object or an object.'
       );
     }
 
@@ -1272,9 +1275,9 @@ class Builder extends EventEmitter {
         alias: alias,
       };
     } else {
-      const order = typeof second === 'string' ? [second] : second;
+      const order = !Array.isArray(second) ? [second] : second;
       let partitions = third || [];
-      partitions = typeof partitions === 'string' ? [partitions] : partitions;
+      partitions = !Array.isArray(partitions) ? [partitions] : partitions;
       analytic = {
         grouping: 'columns',
         type: 'analytic',

diff --git a/lib/query/querycompiler.js b/lib/query/querycompiler.js
@@ -1035,13 +1035,17 @@ class QueryCompiler {
         sql += 'partition by ';
         sql +=
           map(stmt.partitions, function (partition) {
-            return self.formatter.columnize(partition);
+            if (isString(partition)) {
+              return self.formatter.columnize(partition);
+            } else return self.formatter.columnize(partition.column) + (partition.order ? ' ' + partition.order : '');
           }).join(', ') + ' ';
       }
 
       sql += 'order by ';
       sql += map(stmt.order, function (order) {
-        return self.formatter.columnize(order);
+        if (isString(order)) {
+          return self.formatter.columnize(order);
+        } else return self.formatter.columnize(order.column) + (order.order ? ' ' + order.order : '');
       }).join(', ');
     }
 

diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js
@@ -7854,6 +7854,29 @@ describe('QueryBuilder', () => {
     );
   });
 
+  it('row_number with object', function () {
+    testsql(
+      qb()
+        .select('*')
+        .from('accounts')
+        .rowNumber(null, { column: 'email', order: 'asc' }, [
+          { column: 'address', order: 'asc' },
+          'phone',
+        ]),
+      {
+        mssql: {
+          sql: 'select *, row_number() over (partition by [address] asc, [phone] order by [email] asc) from [accounts]',
+        },
+        pg: {
+          sql: 'select *, row_number() over (partition by "address" asc, "phone" order by "email" asc) from "accounts"',
+        },
+        oracledb: {
+          sql: 'select *, row_number() over (partition by "address" asc, "phone" order by "email" asc) from "accounts"',
+        },
+      }
+    );
+  });
+
   it('row_number with string', function () {
     testsql(
       qb().select('*').from('accounts').rowNumber(null, 'email', 'address'),