Repository: knex/knex (js)

Issue:
orderBy inconsistency with positional (numeric) arguments
# Environment

Knex version: 2.4.2
Database + version: Postgres 14.6
OS: MacOS Ventura

# Bug

```
knex('t').select('a', 'b', 'c').orderBy(['a', 'b']).orderBy('c').toString();
  --> select "a", "b", "c" from "t" order by "a" asc, "b" asc, "c" asc    // okay
knex('t').select('a', 'b', 'c').orderBy([1, 2]).orderBy(3).toString();
  --> select "a", "b", "c" from "t" order by 3 asc    // oops, where did 1 and 2 go?
```

Problem: When using positional order-by, the `.orderBy([1, 2])` form is ignored, whereas `.orderBy(1).orderBy(2)` works fine. 


Pull request description:
fix: handle numeric array elements in .orderBy()
Fixes #5525.

Patch:
diff --git a/lib/query/querybuilder.js b/lib/query/querybuilder.js
index 87a8cc3b..94dfa9f8 100644
--- a/lib/query/querybuilder.js
+++ b/lib/query/querybuilder.js
@@ -775,7 +775,7 @@ class Builder extends EventEmitter {
           direction: columnInfo['order'],
           nulls: columnInfo['nulls'],
         });
-      } else if (isString(columnInfo)) {
+      } else if (isString(columnInfo) || isNumber(columnInfo)) {
         this._statements.push({
           grouping: 'order',
           type: 'orderByBasic',


Test patch:
diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js
index c1e86f84..00aff178 100644
--- a/test/unit/query/builder.js
+++ b/test/unit/query/builder.js
@@ -3238,22 +3238,22 @@ describe('QueryBuilder', () => {
       qb()
         .select('*')
         .from('users')
-        .orderBy(['email', { column: 'age', order: 'desc' }]),
+        .orderBy(['email', { column: 'age', order: 'desc' }, 3]),
       {
         mysql: {
-          sql: 'select * from `users` order by `email` asc, `age` desc',
+          sql: 'select * from `users` order by `email` asc, `age` desc, 3 asc',
           bindings: [],
         },
         mssql: {
-          sql: 'select * from [users] order by [email] asc, [age] desc',
+          sql: 'select * from [users] order by [email] asc, [age] desc, 3 asc',
           bindings: [],
         },
         pg: {
-          sql: 'select * from "users" order by "email" asc, "age" desc',
+          sql: 'select * from "users" order by "email" asc, "age" desc, 3 asc',
           bindings: [],
         },
         'pg-redshift': {
-          sql: 'select * from "users" order by "email" asc, "age" desc',
+          sql: 'select * from "users" order by "email" asc, "age" desc, 3 asc',
           bindings: [],
         },
       }
@@ -3455,6 +3455,27 @@ describe('QueryBuilder', () => {
     );
   });
 
+  it('multiple numeric order bys, with and without arrays', () => {
+    testsql(qb().select('*').from('users').orderBy([2, 4]).orderBy(7), {
+      mysql: {
+        sql: 'select * from `users` order by 2 asc, 4 asc, 7 asc',
+        bindings: [],
+      },
+      mssql: {
+        sql: 'select * from [users] order by 2 asc, 4 asc, 7 asc',
+        bindings: [],
+      },
+      pg: {
+        sql: 'select * from "users" order by 2 asc, 4 asc, 7 asc',
+        bindings: [],
+      },
+      'pg-redshift': {
+        sql: 'select * from "users" order by 2 asc, 4 asc, 7 asc',
+        bindings: [],
+      },
+    });
+  });
+
   it('havings', () => {
     testsql(qb().select('*').from('users').having('email', '>', 1), {
       mysql: 'select * from `users` having `email` > ?',
@@ -9448,28 +9469,23 @@ describe('QueryBuilder', () => {
         .comment('Added comment 2'),
       {
         mysql: {
-          sql:
-            '/* Added comment 1 */ /* Added comment 2 */ select * from `testtable`',
+          sql: '/* Added comment 1 */ /* Added comment 2 */ select * from `testtable`',
           bindings: [],
         },
         oracledb: {
-          sql:
-            '/* Added comment 1 */ /* Added comment 2 */ select * from "testtable"',
+          sql: '/* Added comment 1 */ /* Added comment 2 */ select * from "testtable"',
           bindings: [],
         },
         mssql: {
-          sql:
-            '/* Added comment 1 */ /* Added comment 2 */ select * from [testtable]',
+          sql: '/* Added comment 1 */ /* Added comment 2 */ select * from [testtable]',
           bindings: [],
         },
         pg: {
-          sql:
-            '/* Added comment 1 */ /* Added comment 2 */ select * from "testtable"',
+          sql: '/* Added comment 1 */ /* Added comment 2 */ select * from "testtable"',
           bindings: [],
         },
         'pg-redshift': {
-          sql:
-            '/* Added comment 1 */ /* Added comment 2 */ select * from "testtable"',
+          sql: '/* Added comment 1 */ /* Added comment 2 */ select * from "testtable"',
           bindings: [],
         },
       }
@@ -9478,33 +9494,28 @@ describe('QueryBuilder', () => {
 
   it('#1982 (2) - should throw error on non string', () => {
     try {
-      testsql(
-        qb()
-          .from('testtable')
-          .comment({ prop: 'val' }),
-        {
-          mysql: {
-            sql: '',
-            bindings: [],
-          },
-          oracledb: {
-            sql: '',
-            bindings: [],
-          },
-          mssql: {
-            sql: '',
-            bindings: [],
-          },
-          pg: {
-            sql: '',
-            bindings: [],
-          },
-          'pg-redshift': {
-            sql: '',
-            bindings: [],
-          },
-        }
-      );
+      testsql(qb().from('testtable').comment({ prop: 'val' }), {
+        mysql: {
+          sql: '',
+          bindings: [],
+        },
+        oracledb: {
+          sql: '',
+          bindings: [],
+        },
+        mssql: {
+          sql: '',
+          bindings: [],
+        },
+        pg: {
+          sql: '',
+          bindings: [],
+        },
+        'pg-redshift': {
+          sql: '',
+          bindings: [],
+        },
+      });
       expect(true).to.equal(
         false,
         'Expected to throw error in compilation about non-string'
@@ -9516,33 +9527,28 @@ describe('QueryBuilder', () => {
 
   it('#1982 (3) - should throw error when there is subcomments', () => {
     try {
-      testsql(
-        qb()
-          .from('testtable')
-          .comment('/* Hello world'),
-        {
-          mysql: {
-            sql: '',
-            bindings: [],
-          },
-          oracledb: {
-            sql: '',
-            bindings: [],
-          },
-          mssql: {
-            sql: '',
-            bindings: [],
-          },
-          pg: {
-            sql: '',
-            bindings: [],
-          },
-          'pg-redshift': {
-            sql: '',
-            bindings: [],
-          },
-        }
-      );
+      testsql(qb().from('testtable').comment('/* Hello world'), {
+        mysql: {
+          sql: '',
+          bindings: [],
+        },
+        oracledb: {
+          sql: '',
+          bindings: [],
+        },
+        mssql: {
+          sql: '',
+          bindings: [],
+        },
+        pg: {
+          sql: '',
+          bindings: [],
+        },
+        'pg-redshift': {
+          sql: '',
+          bindings: [],
+        },
+      });
       expect(true).to.equal(
         false,
         'Expected to throw error in compilation about non-string'
@@ -9554,33 +9560,28 @@ describe('QueryBuilder', () => {
 
   it('#1982 (4) - should throw error when there is question mark', () => {
     try {
-      testsql(
-        qb()
-          .from('testtable')
-          .comment('?'),
-        {
-          mysql: {
-            sql: '',
-            bindings: [],
-          },
-          oracledb: {
-            sql: '',
-            bindings: [],
-          },
-          mssql: {
-            sql: '',
-            bindings: [],
-          },
-          pg: {
-            sql: '',
-            bindings: [],
-          },
-          'pg-redshift': {
-            sql: '',
-            bindings: [],
-          },
-        }
-      );
+      testsql(qb().from('testtable').comment('?'), {
+        mysql: {
+          sql: '',
+          bindings: [],
+        },
+        oracledb: {
+          sql: '',
+          bindings: [],
+        },
+        mssql: {
+          sql: '',
+          bindings: [],
+        },
+        pg: {
+          sql: '',
+          bindings: [],
+        },
+        'pg-redshift': {
+          sql: '',
+          bindings: [],
+        },
+      });
       expect(true).to.equal(
         false,
         'Expected to throw error in compilation about non-string'