From 150fb08bb24de2094afd8019b033f999d84bd972 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Fri, 14 Dec 2018 17:46:58 -0200
Subject: [PATCH 01/19] Implement "skip locked" and "nowait" for Postgres
 dialect

---
 src/dialects/postgres/query/compiler.js |  8 +++
 src/query/builder.js                    | 36 ++++++++++++
 src/query/compiler.js                   |  8 +++
 test/integration/builder/selects.js     | 78 +++++++++++++++++++++++++
 test/unit/query/builder.js              | 34 +++++++++++
 5 files changed, 164 insertions(+)

diff --git a/src/dialects/postgres/query/compiler.js b/src/dialects/postgres/query/compiler.js
index 33af1910cc..71b1f44161 100644
--- a/src/dialects/postgres/query/compiler.js
+++ b/src/dialects/postgres/query/compiler.js
@@ -102,6 +102,14 @@ assign(QueryCompiler_PG.prototype, {
     );
   },
 
+  skipLocked() {
+    return 'skip locked';
+  },
+
+  noWait() {
+    return 'nowait';
+  },
+
   // Compiles a columnInfo query
   columnInfo() {
     const column = this.single.columnInfo;
diff --git a/src/query/builder.js b/src/query/builder.js
index 3151876a65..8e09ddfec5 100644
--- a/src/query/builder.js
+++ b/src/query/builder.js
@@ -1061,6 +1061,42 @@ assign(Builder.prototype, {
     return this;
   },
 
+  // Skips locked rows when using a lock constraint.
+  skipLocked() {
+    const { _method } = this;
+    if (!includes(['pluck', 'first', 'select'], _method)) {
+      throw new Error(`Cannot chain .skipLocked() on "${_method}" query!`);
+    }
+    if (!includes(['forShare', 'forUpdate'], this._single.lock)) {
+      throw new Error(
+        '.skipLocked() can only be used after a call to .forShare() or .forUpdate()!'
+      );
+    }
+    if (this._single.waitMode) {
+      throw new Error('.skipLocked() cannot be used with .noWait()!');
+    }
+    this._single.waitMode = 'skipLocked';
+    return this;
+  },
+
+  // Causes error when acessing a locked row instead of waiting for it to be released.
+  noWait() {
+    const { _method } = this;
+    if (!includes(['pluck', 'first', 'select'], _method)) {
+      throw new Error(`Cannot chain .noWait() on "${_method}" query!`);
+    }
+    if (!includes(['forShare', 'forUpdate'], this._single.lock)) {
+      throw new Error(
+        '.noWait() can only be used after a call to .forShare() or .forUpdate()!'
+      );
+    }
+    if (this._single.waitMode) {
+      throw new Error('.noWait() cannot be used with .skipLocked()!');
+    }
+    this._single.waitMode = 'noWait';
+    return this;
+  },
+
   // Takes a JS object of methods to call and calls them
   fromJS(obj) {
     each(obj, (val, key) => {
diff --git a/src/query/compiler.js b/src/query/compiler.js
index f261231ac1..7f5415c2ae 100644
--- a/src/query/compiler.js
+++ b/src/query/compiler.js
@@ -48,6 +48,7 @@ const components = [
   'limit',
   'offset',
   'lock',
+  'waitMode',
 ];
 
 assign(QueryCompiler.prototype, {
@@ -542,6 +543,13 @@ assign(QueryCompiler.prototype, {
     }
   },
 
+  // Compiles the wait mode on the locks.
+  waitMode() {
+    if (this.single.waitMode) {
+      return this[this.single.waitMode]();
+    }
+  },
+
   // On Clause
   // ------
 
diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index 3b743563db..1f3442b3d2 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1225,5 +1225,83 @@ module.exports = function(knex) {
             });
         });
     });
+
+    it('forUpdate().skipLocked() should return the first non-locked row', function() {
+      if (knex.client.driverName === 'sqlite3') {
+        return;
+      }
+      const rowName = 'row for skipLocked() test';
+      return knex('test_default_table')
+        .insert([
+          { string: rowName, tinyint: 1 },
+          { string: rowName, tinyint: 2 },
+        ])
+        .then(() => {
+          return knex
+            .transaction((trx) => {
+              // select and lock only the first row from this test
+              return trx('test_default_table')
+                .where({ string: rowName })
+                .orderBy('tinyint', 'asc')
+                .first()
+                .forUpdate()
+                .then((res) => {
+                  // try to lock the second row
+                  return knex('test_default_table')
+                    .where({ string: rowName })
+                    .orderBy('tinyint', 'asc')
+                    .forUpdate()
+                    .skipLocked()
+                    .first();
+                });
+            })
+            .then((res) => {
+              expect(res.tinyint).to.equal(2);
+            });
+        });
+    });
+
+    it('forUpdate().noWait() should reject immediately when a row is locked', function() {
+      if (knex.client.driverName === 'sqlite3') {
+        return;
+      }
+      const rowName = 'row for noWait() test';
+      return knex('test_default_table')
+        .insert([
+          { string: rowName, tinyint: 1 },
+          { string: rowName, tinyint: 2 },
+        ])
+        .then(() => {
+          return knex
+            .transaction((trx) => {
+              // select and lock only the first row from this test
+              return trx('test_default_table')
+                .where({ string: rowName })
+                .orderBy('tinyint', 'asc')
+                .first()
+                .forUpdate()
+                .then((res) => {
+                  // try to lock the second row
+                  return knex('test_default_table')
+                    .where({ string: rowName })
+                    .orderBy('tinyint', 'asc')
+                    .forUpdate()
+                    .noWait()
+                    .first();
+                });
+            })
+            .then((res) => {
+              expect(
+                'The query should have been cancelled when trying to select a locked row'
+              ).to.be.false;
+            })
+            .catch((err) => {
+              // TODO: handle errors from more databases
+              if (knex.client.driverName === 'pg') {
+                expect(err.message).to.contain('could not obtain lock on row');
+              }
+            });
+        });
+    });
   });
 };
diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js
index 218670a87d..152d91fd45 100644
--- a/test/unit/query/builder.js
+++ b/test/unit/query/builder.js
@@ -6377,6 +6377,40 @@ describe('QueryBuilder', function() {
     );
   });
 
+  it('lock for update with skip locked #1937', function() {
+    testsql(
+      qb()
+        .select('*')
+        .from('foo')
+        .first()
+        .forUpdate()
+        .skipLocked(),
+      {
+        pg: {
+          sql: 'select * from "foo" limit ? for update skip locked',
+          bindings: [1],
+        },
+      }
+    );
+  });
+
+  it('lock for update with nowait #1937', function() {
+    testsql(
+      qb()
+        .select('*')
+        .from('foo')
+        .first()
+        .forUpdate()
+        .noWait(),
+      {
+        pg: {
+          sql: 'select * from "foo" limit ? for update nowait',
+          bindings: [1],
+        },
+      }
+    );
+  });
+
   it('allows insert values of sub-select, #121', function() {
     testsql(
       qb()

From b9dd2c67b66697058acd314ac3014ae051882767 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Wed, 19 Dec 2018 12:42:25 -0200
Subject: [PATCH 02/19] Fix error with duplicate skipLocked or noWait call

---
 src/query/builder.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/query/builder.js b/src/query/builder.js
index 8e09ddfec5..9c748d6d50 100644
--- a/src/query/builder.js
+++ b/src/query/builder.js
@@ -1072,7 +1072,7 @@ assign(Builder.prototype, {
         '.skipLocked() can only be used after a call to .forShare() or .forUpdate()!'
       );
     }
-    if (this._single.waitMode) {
+    if (this._single.waitMode === 'noWait') {
       throw new Error('.skipLocked() cannot be used with .noWait()!');
     }
     this._single.waitMode = 'skipLocked';
@@ -1090,7 +1090,7 @@ assign(Builder.prototype, {
         '.noWait() can only be used after a call to .forShare() or .forUpdate()!'
       );
     }
-    if (this._single.waitMode) {
+    if (this._single.waitMode === 'skipLocked') {
       throw new Error('.noWait() cannot be used with .skipLocked()!');
     }
     this._single.waitMode = 'noWait';

From 21775f9f55a0b6ad3c237e34f78ed878cd6cb20f Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Wed, 19 Dec 2018 12:53:01 -0200
Subject: [PATCH 03/19] Fix noWait test swallowing errors on non-postgres
 databases

---
 test/integration/builder/selects.js | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index 1f3442b3d2..190c86974f 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1296,9 +1296,15 @@ module.exports = function(knex) {
               ).to.be.false;
             })
             .catch((err) => {
-              // TODO: handle errors from more databases
-              if (knex.client.driverName === 'pg') {
-                expect(err.message).to.contain('could not obtain lock on row');
+              switch (knex.client.driverName) {
+                case 'pg':
+                  expect(err.message).to.contain(
+                    'could not obtain lock on row'
+                  );
+                  break;
+                default:
+                  // unsupported database
+                  throw err;
               }
             });
         });

From 0f13016a7ba2dc5405b34080bdaeb2c324276ea7 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Wed, 19 Dec 2018 13:33:40 -0200
Subject: [PATCH 04/19] Implement skipLocked and noWait for mysql-based clients

---
 src/dialects/mysql/query/compiler.js | 10 ++++++++++
 test/integration/builder/selects.js  |  4 ++++
 2 files changed, 14 insertions(+)

diff --git a/src/dialects/mysql/query/compiler.js b/src/dialects/mysql/query/compiler.js
index 9e34e19044..3f235fd267 100644
--- a/src/dialects/mysql/query/compiler.js
+++ b/src/dialects/mysql/query/compiler.js
@@ -39,6 +39,16 @@ assign(QueryCompiler_MySQL.prototype, {
     return 'lock in share mode';
   },
 
+  // Only supported on MySQL 8.0+
+  skipLocked() {
+    return 'skip locked';
+  },
+
+  // Supported on MySQL 8.0+ and MariaDB 10.3.0+
+  noWait() {
+    return 'nowait';
+  },
+
   // Compiles a `columnInfo` query.
   columnInfo() {
     const column = this.single.columnInfo;
diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index 190c86974f..179840341e 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1302,6 +1302,10 @@ module.exports = function(knex) {
                     'could not obtain lock on row'
                   );
                   break;
+                case 'mysql':
+                case 'mysql2':
+                  expect(err.message).to.contain('Lock wait timeout exceeded');
+                  break;
                 default:
                   // unsupported database
                   throw err;

From d73663a2cb1e2d1f293f9be027acf29ca6ca7dee Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Tue, 8 Jan 2019 10:36:04 -0200
Subject: [PATCH 05/19] Disable skipLocked test on MySQL

---
 test/integration/builder/selects.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index 179840341e..7badb35f06 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1227,9 +1227,11 @@ module.exports = function(knex) {
     });
 
     it('forUpdate().skipLocked() should return the first non-locked row', function() {
-      if (knex.client.driverName === 'sqlite3') {
+      // TODO: enable this test on MySQL when updating test suite to MySQL 8.0
+      if (knex.client.driverName !== 'pg') {
         return;
       }
+
       const rowName = 'row for skipLocked() test';
       return knex('test_default_table')
         .insert([

From dbac68e113496e56b33e80e8c85a96da6377e69c Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Tue, 8 Jan 2019 10:37:14 -0200
Subject: [PATCH 06/19] Restrict noWait test to supported databases

---
 test/integration/builder/selects.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index 7badb35f06..63850d39d3 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1264,9 +1264,15 @@ module.exports = function(knex) {
     });
 
     it('forUpdate().noWait() should reject immediately when a row is locked', function() {
-      if (knex.client.driverName === 'sqlite3') {
+      // enable test only on supported databases
+      if (
+        knex.client.driverName !== 'pg' &&
+        knex.client.driverName !== 'mysql' &&
+        knex.client.driverName !== 'mysql2'
+      ) {
         return;
       }
+
       const rowName = 'row for noWait() test';
       return knex('test_default_table')
         .insert([

From 71fd14b98e096736d9a968b94d2d68bc70adbf94 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Tue, 8 Jan 2019 10:42:17 -0200
Subject: [PATCH 07/19] Update knex.d.ts with skipLocked and noWait

---
 types/knex.d.ts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/types/knex.d.ts b/types/knex.d.ts
index a553d2a0d9..3cb8614807 100644
--- a/types/knex.d.ts
+++ b/types/knex.d.ts
@@ -464,6 +464,10 @@ declare namespace Knex {
     forShare(...tableNames: string[]): QueryBuilder;
     forShare(tableNames: string[]): QueryBuilder;
 
+    skipLocked(): QueryBuilder;
+
+    noWait(): QueryBuilder;
+
     toSQL(): Sql;
 
     on(event: string, callback: Function): QueryBuilder;

From de3b4cc04b68ab1bfa145c96835c97ab10bfdf15 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Tue, 8 Jan 2019 13:10:20 -0200
Subject: [PATCH 08/19] Adjust noWait test for MySQL 8.0

---
 test/integration/builder/selects.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index 63850d39d3..eb0b413f2f 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1312,7 +1312,13 @@ module.exports = function(knex) {
                   break;
                 case 'mysql':
                 case 'mysql2':
-                  expect(err.message).to.contain('Lock wait timeout exceeded');
+                  // mysql
+                  expect(err.message).to.contain(
+                    'lock(s) could not be acquired immediately'
+                  );
+                  // mariadb
+                  // TODO: detect if test is being run on mysql or mariadb to check for the correct error message
+                  // expect(err.message).to.contain('Lock wait timeout exceeded');
                   break;
                 default:
                   // unsupported database

From 94de977d334c9be14432baf6a0da61afd85a2fa9 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Tue, 8 Jan 2019 13:11:03 -0200
Subject: [PATCH 09/19] Disable noWait test on MySQL

---
 test/integration/builder/selects.js | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index eb0b413f2f..764f0f12dd 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1264,12 +1264,8 @@ module.exports = function(knex) {
     });
 
     it('forUpdate().noWait() should reject immediately when a row is locked', function() {
-      // enable test only on supported databases
-      if (
-        knex.client.driverName !== 'pg' &&
-        knex.client.driverName !== 'mysql' &&
-        knex.client.driverName !== 'mysql2'
-      ) {
+      // TODO: enable this test on MySQL when updating test suite to MySQL 8.0
+      if (knex.client.driverName !== 'pg') {
         return;
       }
 

From 7db84fa94ca567001f94349e9684975c423713b5 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Tue, 8 Jan 2019 13:33:38 -0200
Subject: [PATCH 10/19] Increase query builder test coverage for MySQL

---
 test/unit/query/builder.js | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js
index 152d91fd45..b96bf634ba 100644
--- a/test/unit/query/builder.js
+++ b/test/unit/query/builder.js
@@ -6386,6 +6386,10 @@ describe('QueryBuilder', function() {
         .forUpdate()
         .skipLocked(),
       {
+        mysql: {
+          sql: 'select * from `foo` limit ? for update skip locked',
+          bindings: [1],
+        },
         pg: {
           sql: 'select * from "foo" limit ? for update skip locked',
           bindings: [1],
@@ -6403,6 +6407,10 @@ describe('QueryBuilder', function() {
         .forUpdate()
         .noWait(),
       {
+        mysql: {
+          sql: 'select * from `foo` limit ? for update nowait',
+          bindings: [1],
+        },
         pg: {
           sql: 'select * from "foo" limit ? for update nowait',
           bindings: [1],

From e49c4b73031ff37bb57965baf5d72263fc496834 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Sun, 10 Mar 2019 15:19:24 -0300
Subject: [PATCH 11/19] Remove MySQL support for skipLocked() and noWait()

Reasoning: while this feature is useful, it's untestable without the proper framework for testing on MySQL 8.0, and testing MySQL and MariaDB separately.

See: https://github.com/tgriesser/knex/pull/2961
---
 src/dialects/mysql/query/compiler.js | 10 ----------
 test/unit/query/builder.js           |  8 --------
 2 files changed, 18 deletions(-)

diff --git a/src/dialects/mysql/query/compiler.js b/src/dialects/mysql/query/compiler.js
index 3f235fd267..9e34e19044 100644
--- a/src/dialects/mysql/query/compiler.js
+++ b/src/dialects/mysql/query/compiler.js
@@ -39,16 +39,6 @@ assign(QueryCompiler_MySQL.prototype, {
     return 'lock in share mode';
   },
 
-  // Only supported on MySQL 8.0+
-  skipLocked() {
-    return 'skip locked';
-  },
-
-  // Supported on MySQL 8.0+ and MariaDB 10.3.0+
-  noWait() {
-    return 'nowait';
-  },
-
   // Compiles a `columnInfo` query.
   columnInfo() {
     const column = this.single.columnInfo;
diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js
index b96bf634ba..152d91fd45 100644
--- a/test/unit/query/builder.js
+++ b/test/unit/query/builder.js
@@ -6386,10 +6386,6 @@ describe('QueryBuilder', function() {
         .forUpdate()
         .skipLocked(),
       {
-        mysql: {
-          sql: 'select * from `foo` limit ? for update skip locked',
-          bindings: [1],
-        },
         pg: {
           sql: 'select * from "foo" limit ? for update skip locked',
           bindings: [1],
@@ -6407,10 +6403,6 @@ describe('QueryBuilder', function() {
         .forUpdate()
         .noWait(),
       {
-        mysql: {
-          sql: 'select * from `foo` limit ? for update nowait',
-          bindings: [1],
-        },
         pg: {
           sql: 'select * from "foo" limit ? for update nowait',
           bindings: [1],

From f32488f68f70a668ba212609fc898eeb036d4b63 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Tue, 12 Mar 2019 17:26:41 -0300
Subject: [PATCH 12/19] Add errors for databases that don't support
 skipLocked() and noWait()

---
 src/query/compiler.js | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/query/compiler.js b/src/query/compiler.js
index 7f5415c2ae..1d8ef0d8c0 100644
--- a/src/query/compiler.js
+++ b/src/query/compiler.js
@@ -550,6 +550,16 @@ assign(QueryCompiler.prototype, {
     }
   },
 
+  // Fail on unsupported databases
+  skipLocked() {
+    throw new Error('.skipLocked() is currently only supported on PostgreSQL');
+  },
+
+  // Fail on unsupported databases
+  noWait() {
+    throw new Error('.noWait() is currently only supported on PostgreSQL');
+  },
+
   // On Clause
   // ------
 

From dcaff9edd2d1249db5a007c26f4bb6b10bc8db99 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Fri, 21 Jun 2019 09:34:22 -0300
Subject: [PATCH 13/19] Update test comments

---
 test/integration/builder/selects.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index af249a67df..a567bd56b4 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1263,14 +1263,14 @@ module.exports = function(knex) {
         .then(() => {
           return knex
             .transaction((trx) => {
-              // select and lock only the first row from this test
+              // lock only the first row from this test
               return trx('test_default_table')
                 .where({ string: rowName })
                 .orderBy('tinyint', 'asc')
                 .first()
                 .forUpdate()
                 .then((res) => {
-                  // try to lock the second row
+                  // try to lock the next available row
                   return knex('test_default_table')
                     .where({ string: rowName })
                     .orderBy('tinyint', 'asc')

From 18cccab4bf06188488146ffbb05ecf7087392c28 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Fri, 21 Jun 2019 09:51:36 -0300
Subject: [PATCH 14/19] Restore MySQL tests for #2961

---
 src/dialects/mysql/query/compiler.js | 10 ++++++++++
 src/query/compiler.js                |  8 ++++++--
 test/integration/builder/selects.js  | 12 ++++++++----
 test/unit/query/builder.js           |  8 ++++++++
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/dialects/mysql/query/compiler.js b/src/dialects/mysql/query/compiler.js
index e517a84cdb..2565c3ff60 100644
--- a/src/dialects/mysql/query/compiler.js
+++ b/src/dialects/mysql/query/compiler.js
@@ -48,6 +48,16 @@ assign(QueryCompiler_MySQL.prototype, {
     return 'lock in share mode';
   },
 
+  // Only supported on MySQL 8.0+
+  skipLocked() {
+    return 'skip locked';
+  },
+
+  // Supported on MySQL 8.0+ and MariaDB 10.3.0+
+  noWait() {
+    return 'nowait';
+  },
+
   // Compiles a `columnInfo` query.
   columnInfo() {
     const column = this.single.columnInfo;
diff --git a/src/query/compiler.js b/src/query/compiler.js
index 7f47f6e7c3..31e175239c 100644
--- a/src/query/compiler.js
+++ b/src/query/compiler.js
@@ -552,12 +552,16 @@ assign(QueryCompiler.prototype, {
 
   // Fail on unsupported databases
   skipLocked() {
-    throw new Error('.skipLocked() is currently only supported on PostgreSQL');
+    throw new Error(
+      '.skipLocked() is currently only supported on MySQL 8.0+ and PostgreSQL 9.5+'
+    );
   },
 
   // Fail on unsupported databases
   noWait() {
-    throw new Error('.noWait() is currently only supported on PostgreSQL');
+    throw new Error(
+      '.noWait() is currently only supported on MySQL 8.0+, MariaDB 10.3.0+ and PostgreSQL 9.5+'
+    );
   },
 
   // On Clause
diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index a567bd56b4..b4e39a0cee 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1249,8 +1249,10 @@ module.exports = function(knex) {
     });
 
     it('forUpdate().skipLocked() should return the first non-locked row', function() {
-      // TODO: enable this test on MySQL when updating test suite to MySQL 8.0
-      if (knex.client.driverName !== 'pg') {
+      if (
+        knex.client.driverName !== 'pg' &&
+        knex.client.driverName !== 'mysql'
+      ) {
         return;
       }
 
@@ -1286,8 +1288,10 @@ module.exports = function(knex) {
     });
 
     it('forUpdate().noWait() should reject immediately when a row is locked', function() {
-      // TODO: enable this test on MySQL when updating test suite to MySQL 8.0
-      if (knex.client.driverName !== 'pg') {
+      if (
+        knex.client.driverName !== 'pg' &&
+        knex.client.driverName !== 'mysql'
+      ) {
         return;
       }
 
diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js
index 015cad42b6..51e58b1666 100644
--- a/test/unit/query/builder.js
+++ b/test/unit/query/builder.js
@@ -7007,6 +7007,10 @@ describe('QueryBuilder', function() {
         .forUpdate()
         .skipLocked(),
       {
+        mysql: {
+          sql: 'select * from `foo` limit ? for update skip locked',
+          bindings: [1],
+        },
         pg: {
           sql: 'select * from "foo" limit ? for update skip locked',
           bindings: [1],
@@ -7024,6 +7028,10 @@ describe('QueryBuilder', function() {
         .forUpdate()
         .noWait(),
       {
+        mysql: {
+          sql: 'select * from `foo` limit ? for update nowait',
+          bindings: [1],
+        },
         pg: {
           sql: 'select * from "foo" limit ? for update nowait',
           bindings: [1],

From e9b152d5fde16683407122b040edc1b98a14411a Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Fri, 21 Jun 2019 10:31:27 -0300
Subject: [PATCH 15/19] Simplify query builder checks on first(), noWait() and
 skipLocked()

---
 src/query/builder.js | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/src/query/builder.js b/src/query/builder.js
index 93f6acd8b1..ed08a13bc2 100644
--- a/src/query/builder.js
+++ b/src/query/builder.js
@@ -950,10 +950,8 @@ assign(Builder.prototype, {
   // Sets the values for a `select` query, informing that only the first
   // row should be returned (limit 1).
   first() {
-    const { _method } = this;
-
-    if (!includes(['pluck', 'first', 'select'], _method)) {
-      throw new Error(`Cannot chain .first() on "${_method}" query!`);
+    if (!this._isSelectQuery()) {
+      throw new Error(`Cannot chain .first() on "${this._method}" query!`);
     }
 
     const args = new Array(arguments.length);
@@ -1095,17 +1093,16 @@ assign(Builder.prototype, {
 
   // Skips locked rows when using a lock constraint.
   skipLocked() {
-    const { _method } = this;
-    if (!includes(['pluck', 'first', 'select'], _method)) {
-      throw new Error(`Cannot chain .skipLocked() on "${_method}" query!`);
+    if (!this._isSelectQuery()) {
+      throw new Error(`Cannot chain .skipLocked() on "${this._method}" query!`);
     }
-    if (!includes(['forShare', 'forUpdate'], this._single.lock)) {
+    if (!this._hasLockMode()) {
       throw new Error(
         '.skipLocked() can only be used after a call to .forShare() or .forUpdate()!'
       );
     }
     if (this._single.waitMode === 'noWait') {
-      throw new Error('.skipLocked() cannot be used with .noWait()!');
+      throw new Error('.skipLocked() cannot be used together with .noWait()!');
     }
     this._single.waitMode = 'skipLocked';
     return this;
@@ -1113,17 +1110,16 @@ assign(Builder.prototype, {
 
   // Causes error when acessing a locked row instead of waiting for it to be released.
   noWait() {
-    const { _method } = this;
-    if (!includes(['pluck', 'first', 'select'], _method)) {
-      throw new Error(`Cannot chain .noWait() on "${_method}" query!`);
+    if (!this._isSelectQuery()) {
+      throw new Error(`Cannot chain .noWait() on "${this._method}" query!`);
     }
-    if (!includes(['forShare', 'forUpdate'], this._single.lock)) {
+    if (!this._hasLockMode()) {
       throw new Error(
         '.noWait() can only be used after a call to .forShare() or .forUpdate()!'
       );
     }
     if (this._single.waitMode === 'skipLocked') {
-      throw new Error('.noWait() cannot be used with .skipLocked()!');
+      throw new Error('.noWait() cannot be used together with .skipLocked()!');
     }
     this._single.waitMode = 'noWait';
     return this;
@@ -1215,6 +1211,16 @@ assign(Builder.prototype, {
   _clearGrouping(grouping) {
     this._statements = reject(this._statements, { grouping });
   },
+
+  // Helper function that checks if the builder will emit a select query
+  _isSelectQuery() {
+    return includes(['pluck', 'first', 'select'], this._method);
+  },
+
+  // Helper function that checks if the query has a lock mode set
+  _hasLockMode() {
+    return includes(['forShare', 'forUpdate'], this._single.lock);
+  },
 });
 
 Object.defineProperty(Builder.prototype, 'or', {

From 2a92c9da451964492ab8cc288533d503ba511ff6 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Fri, 21 Jun 2019 10:52:50 -0300
Subject: [PATCH 16/19] Add some failing tests for PR #2961

---
 test/unit/query/builder.js | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js
index 51e58b1666..cf38dc103f 100644
--- a/test/unit/query/builder.js
+++ b/test/unit/query/builder.js
@@ -7040,6 +7040,44 @@ describe('QueryBuilder', function() {
     );
   });
 
+  it('noWait and skipLocked require a lock mode to be set', function() {
+    expect(function() {
+      qb()
+        .select('*')
+        .noWait()
+        .toString();
+    }).to.throw(
+      '.noWait() can only be used after a call to .forShare() or .forUpdate()!'
+    );
+    expect(function() {
+      qb()
+        .select('*')
+        .skipLocked()
+        .toString();
+    }).to.throw(
+      '.skipLocked() can only be used after a call to .forShare() or .forUpdate()!'
+    );
+  });
+
+  it('skipLocked conflicts with noWait and vice-versa', function() {
+    expect(function() {
+      qb()
+        .select('*')
+        .forUpdate()
+        .noWait()
+        .skipLocked()
+        .toString();
+    }).to.throw('.skipLocked() cannot be used together with .noWait()!');
+    expect(function() {
+      qb()
+        .select('*')
+        .forUpdate()
+        .skipLocked()
+        .noWait()
+        .toString();
+    }).to.throw('.noWait() cannot be used together with .skipLocked()!');
+  });
+
   it('allows insert values of sub-select, #121', function() {
     testsql(
       qb()

From 6eb290a199cb64588a10949f9463e0c7f069e37f Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Fri, 21 Jun 2019 18:01:54 -0300
Subject: [PATCH 17/19] Replace broken test on MySQL with an extra test for
 #2961

---
 test/integration/builder/selects.js | 46 +++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index b4e39a0cee..271bcdb8d2 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1248,15 +1248,13 @@ module.exports = function(knex) {
         });
     });
 
-    it('forUpdate().skipLocked() should return the first non-locked row', function() {
-      if (
-        knex.client.driverName !== 'pg' &&
-        knex.client.driverName !== 'mysql'
-      ) {
+    it('forUpdate().skipLocked() with order by should return the first non-locked row', function() {
+      // Note: this test doesn't work properly on MySQL - see https://bugs.mysql.com/bug.php?id=67745
+      if (knex.client.driverName !== 'pg') {
         return;
       }
 
-      const rowName = 'row for skipLocked() test';
+      const rowName = 'row for skipLocked() test #1';
       return knex('test_default_table')
         .insert([
           { string: rowName, tinyint: 1 },
@@ -1287,6 +1285,42 @@ module.exports = function(knex) {
         });
     });
 
+    it('forUpdate().skipLocked() should return an empty set when all rows are locked', function() {
+      if (
+        knex.client.driverName !== 'pg' &&
+        knex.client.driverName !== 'mysql'
+      ) {
+        return;
+      }
+
+      const rowName = 'row for skipLocked() test #2';
+      return knex('test_default_table')
+        .insert([
+          { string: rowName, tinyint: 1 },
+          { string: rowName, tinyint: 2 },
+        ])
+        .then(() => {
+          return knex
+            .transaction((trx) => {
+              // lock all of the test rows
+              return trx('test_default_table')
+                .where({ string: rowName })
+                .forUpdate()
+                .then((res) => {
+                  // try to aquire the lock on at least one more row (which isn't available)
+                  return knex('test_default_table')
+                    .where({ string: rowName })
+                    .forUpdate()
+                    .skipLocked()
+                    .limit(1);
+                });
+            })
+            .then((res) => {
+              expect(res).to.be.empty;
+            });
+        });
+    });
+
     it('forUpdate().noWait() should reject immediately when a row is locked', function() {
       if (
         knex.client.driverName !== 'pg' &&

From b22cd7b162f39bf4cc55dbe78962d976aec4cbb2 Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Sat, 29 Jun 2019 19:22:18 -0300
Subject: [PATCH 18/19] Adjust the #2961 tests to use async/await

---
 test/integration/builder/selects.js | 206 +++++++++++++---------------
 1 file changed, 98 insertions(+), 108 deletions(-)

diff --git a/test/integration/builder/selects.js b/test/integration/builder/selects.js
index 271bcdb8d2..4c74ad015c 100644
--- a/test/integration/builder/selects.js
+++ b/test/integration/builder/selects.js
@@ -1248,44 +1248,40 @@ module.exports = function(knex) {
         });
     });
 
-    it('forUpdate().skipLocked() with order by should return the first non-locked row', function() {
+    it('forUpdate().skipLocked() with order by should return the first non-locked row', async function() {
       // Note: this test doesn't work properly on MySQL - see https://bugs.mysql.com/bug.php?id=67745
       if (knex.client.driverName !== 'pg') {
         return;
       }
 
       const rowName = 'row for skipLocked() test #1';
-      return knex('test_default_table')
-        .insert([
-          { string: rowName, tinyint: 1 },
-          { string: rowName, tinyint: 2 },
-        ])
-        .then(() => {
-          return knex
-            .transaction((trx) => {
-              // lock only the first row from this test
-              return trx('test_default_table')
-                .where({ string: rowName })
-                .orderBy('tinyint', 'asc')
-                .first()
-                .forUpdate()
-                .then((res) => {
-                  // try to lock the next available row
-                  return knex('test_default_table')
-                    .where({ string: rowName })
-                    .orderBy('tinyint', 'asc')
-                    .forUpdate()
-                    .skipLocked()
-                    .first();
-                });
-            })
-            .then((res) => {
-              expect(res.tinyint).to.equal(2);
-            });
-        });
+      await knex('test_default_table').insert([
+        { string: rowName, tinyint: 1 },
+        { string: rowName, tinyint: 2 },
+      ]);
+
+      const res = await knex.transaction(async (trx) => {
+        // lock the first row in the test
+        await trx('test_default_table')
+          .where({ string: rowName })
+          .orderBy('tinyint', 'asc')
+          .first()
+          .forUpdate();
+
+        // try to lock the next available row
+        return await knex('test_default_table')
+          .where({ string: rowName })
+          .orderBy('tinyint', 'asc')
+          .forUpdate()
+          .skipLocked()
+          .first();
+      });
+
+      // assert that we got the second row because the first one was locked
+      expect(res.tinyint).to.equal(2);
     });
 
-    it('forUpdate().skipLocked() should return an empty set when all rows are locked', function() {
+    it('forUpdate().skipLocked() should return an empty set when all rows are locked', async function() {
       if (
         knex.client.driverName !== 'pg' &&
         knex.client.driverName !== 'mysql'
@@ -1294,34 +1290,29 @@ module.exports = function(knex) {
       }
 
       const rowName = 'row for skipLocked() test #2';
-      return knex('test_default_table')
-        .insert([
-          { string: rowName, tinyint: 1 },
-          { string: rowName, tinyint: 2 },
-        ])
-        .then(() => {
-          return knex
-            .transaction((trx) => {
-              // lock all of the test rows
-              return trx('test_default_table')
-                .where({ string: rowName })
-                .forUpdate()
-                .then((res) => {
-                  // try to aquire the lock on at least one more row (which isn't available)
-                  return knex('test_default_table')
-                    .where({ string: rowName })
-                    .forUpdate()
-                    .skipLocked()
-                    .limit(1);
-                });
-            })
-            .then((res) => {
-              expect(res).to.be.empty;
-            });
-        });
+      await knex('test_default_table').insert([
+        { string: rowName, tinyint: 1 },
+        { string: rowName, tinyint: 2 },
+      ]);
+
+      const res = await knex.transaction(async (trx) => {
+        // lock all of the test rows
+        await trx('test_default_table')
+          .where({ string: rowName })
+          .forUpdate();
+
+        // try to aquire the lock on one more row (which isn't available)
+        return await knex('test_default_table')
+          .where({ string: rowName })
+          .forUpdate()
+          .skipLocked()
+          .limit(1);
+      });
+
+      expect(res).to.be.empty;
     });
 
-    it('forUpdate().noWait() should reject immediately when a row is locked', function() {
+    it('forUpdate().noWait() should throw immediately when a row is locked', async function() {
       if (
         knex.client.driverName !== 'pg' &&
         knex.client.driverName !== 'mysql'
@@ -1330,58 +1321,57 @@ module.exports = function(knex) {
       }
 
       const rowName = 'row for noWait() test';
-      return knex('test_default_table')
-        .insert([
-          { string: rowName, tinyint: 1 },
-          { string: rowName, tinyint: 2 },
-        ])
-        .then(() => {
-          return knex
-            .transaction((trx) => {
-              // select and lock only the first row from this test
-              return trx('test_default_table')
-                .where({ string: rowName })
-                .orderBy('tinyint', 'asc')
-                .first()
-                .forUpdate()
-                .then((res) => {
-                  // try to lock the second row
-                  return knex('test_default_table')
-                    .where({ string: rowName })
-                    .orderBy('tinyint', 'asc')
-                    .forUpdate()
-                    .noWait()
-                    .first();
-                });
-            })
-            .then((res) => {
-              expect(
-                'The query should have been cancelled when trying to select a locked row'
-              ).to.be.false;
-            })
-            .catch((err) => {
-              switch (knex.client.driverName) {
-                case 'pg':
-                  expect(err.message).to.contain(
-                    'could not obtain lock on row'
-                  );
-                  break;
-                case 'mysql':
-                case 'mysql2':
-                  // mysql
-                  expect(err.message).to.contain(
-                    'lock(s) could not be acquired immediately'
-                  );
-                  // mariadb
-                  // TODO: detect if test is being run on mysql or mariadb to check for the correct error message
-                  // expect(err.message).to.contain('Lock wait timeout exceeded');
-                  break;
-                default:
-                  // unsupported database
-                  throw err;
-              }
-            });
-        });
+      await knex('test_default_table').insert([
+        { string: rowName, tinyint: 1 },
+        { string: rowName, tinyint: 2 },
+      ]);
+
+      const promise = knex.transaction(async (trx) => {
+        // select and lock only the first row from this test
+        // note: MySQL may lock both rows depending on how the results are fetched
+        await trx('test_default_table')
+          .where({ string: rowName })
+          .orderBy('tinyint', 'asc')
+          .first()
+          .forUpdate();
+
+        // try to lock it again (and fail)
+        await trx('test_default_table')
+          .where({ string: rowName })
+          .orderBy('tinyint', 'asc')
+          .forUpdate()
+          .noWait()
+          .first();
+      });
+
+      // catch the expected errors
+      promise.catch((err) => {
+        switch (knex.client.driverName) {
+          case 'pg':
+            expect(err.message).to.contain('could not obtain lock on row');
+            break;
+          case 'mysql':
+          case 'mysql2':
+            // mysql
+            expect(err.message).to.contain(
+              'lock(s) could not be acquired immediately'
+            );
+            // mariadb
+            // TODO: detect if test is being run on mysql or mariadb to check for the correct error message
+            // expect(err.message).to.contain('Lock wait timeout exceeded');
+            break;
+          default:
+            // unsupported database
+            throw err;
+        }
+      });
+
+      // fail the test if the transaction succeeds
+      promise.then(() => {
+        expect(
+          'The query should have been cancelled when trying to select a locked row with .noWait()'
+        ).to.be.false;
+      });
     });
   });
 };

From c0b1cd426c84476dc31787b298a1aa01db0b91fe Mon Sep 17 00:00:00 2001
From: Ricardo Maes <ricmzn@gmail.com>
Date: Tue, 2 Jul 2019 22:34:36 -0300
Subject: [PATCH 19/19] Move some magic query builder values into constants

---
 src/query/builder.js   | 16 +++++++++-------
 src/query/constants.js | 13 +++++++++++++
 2 files changed, 22 insertions(+), 7 deletions(-)
 create mode 100644 src/query/constants.js

diff --git a/src/query/builder.js b/src/query/builder.js
index ed08a13bc2..ac7db4a5b9 100644
--- a/src/query/builder.js
+++ b/src/query/builder.js
@@ -25,6 +25,8 @@ const {
 } = require('lodash');
 const saveAsyncStack = require('../util/save-async-stack');
 
+const { lockMode, waitMode } = require('./constants');
+
 // Typically called from `knex.builder`,
 // start a new query building chain.
 function Builder(client) {
@@ -1079,14 +1081,14 @@ assign(Builder.prototype, {
 
   // Set a lock for update constraint.
   forUpdate() {
-    this._single.lock = 'forUpdate';
+    this._single.lock = lockMode.forUpdate;
     this._single.lockTables = helpers.normalizeArr.apply(null, arguments);
     return this;
   },
 
   // Set a lock for share constraint.
   forShare() {
-    this._single.lock = 'forShare';
+    this._single.lock = lockMode.forShare;
     this._single.lockTables = helpers.normalizeArr.apply(null, arguments);
     return this;
   },
@@ -1101,10 +1103,10 @@ assign(Builder.prototype, {
         '.skipLocked() can only be used after a call to .forShare() or .forUpdate()!'
       );
     }
-    if (this._single.waitMode === 'noWait') {
+    if (this._single.waitMode === waitMode.noWait) {
       throw new Error('.skipLocked() cannot be used together with .noWait()!');
     }
-    this._single.waitMode = 'skipLocked';
+    this._single.waitMode = waitMode.skipLocked;
     return this;
   },
 
@@ -1118,10 +1120,10 @@ assign(Builder.prototype, {
         '.noWait() can only be used after a call to .forShare() or .forUpdate()!'
       );
     }
-    if (this._single.waitMode === 'skipLocked') {
+    if (this._single.waitMode === waitMode.skipLocked) {
       throw new Error('.noWait() cannot be used together with .skipLocked()!');
     }
-    this._single.waitMode = 'noWait';
+    this._single.waitMode = waitMode.noWait;
     return this;
   },
 
@@ -1219,7 +1221,7 @@ assign(Builder.prototype, {
 
   // Helper function that checks if the query has a lock mode set
   _hasLockMode() {
-    return includes(['forShare', 'forUpdate'], this._single.lock);
+    return includes([lockMode.forShare, lockMode.forUpdate], this._single.lock);
   },
 });
 
diff --git a/src/query/constants.js b/src/query/constants.js
new file mode 100644
index 0000000000..0b080715dd
--- /dev/null
+++ b/src/query/constants.js
@@ -0,0 +1,13 @@
+/**
+ * internal constants, do not use in application code
+ */
+module.exports = {
+  lockMode: {
+    forShare: 'forShare',
+    forUpdate: 'forUpdate',
+  },
+  waitMode: {
+    skipLocked: 'skipLocked',
+    noWait: 'noWait',
+  },
+};