Missing comments on delete, update and insert (#5738)

Co-authored-by: Igor Savin <iselwin@gmail.com>
knex · Nov 28, 2023 · 57419fb · 57419fb
1 parent af0e9c1
commit 57419fb
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 0 deletions.
diff --git a/lib/dialects/mysql/query/mysql-querycompiler.js b/lib/dialects/mysql/query/mysql-querycompiler.js
@@ -27,12 +27,21 @@ class QueryCompiler_MySQL extends QueryCompiler {
 
     this._emptyInsertValue = '() values ()';
   }
+  // Compiles an `delete` allowing comments
+  del() {
+    const sql = super.del();
+    if (sql === '') return sql;
+    const comments = this.comments();
+    return (comments === '' ? '' : comments + ' ') + sql;
+  }
 
   // Compiles an `insert` query, allowing for multiple
   // inserts using a single query statement.
   insert() {
     let sql = super.insert();
     if (sql === '') return sql;
+    const comments = this.comments();
+    sql = (comments === '' ? '' : comments + ' ') + sql;
 
     const { ignore, merge, insert } = this.single;
     if (ignore) sql = sql.replace('insert into', 'insert ignore into');
@@ -93,13 +102,15 @@ class QueryCompiler_MySQL extends QueryCompiler {
 
   // Update method, including joins, wheres, order & limits.
   update() {
+    const comments = this.comments();
     const withSQL = this.with();
     const join = this.join();
     const updates = this._prepUpdate(this.single.update);
     const where = this.where();
     const order = this.order();
     const limit = this.limit();
     return (
+      (comments === '' ? '' : comments + ' ') +
       withSQL +
       `update ${this.tableName}` +
       (join ? ` ${join}` : '') +

diff --git a/test/integration/query/deletes.js b/test/integration/query/deletes.js
@@ -86,6 +86,21 @@ module.exports = function (knex) {
         });
     });
 
+    it('#5738 should handle deletes with comments', function () {
+      return knex('accounts')
+        .where('id', 1)
+        .del()
+        .comment('removing acccount')
+        .testSql(function (tester) {
+          tester(
+            'mysql',
+            '/* removing acccount */ delete from `accounts` where `id` = ?',
+            [1],
+            0
+          );
+        });
+    });
+
     describe('Delete with join', function () {
       it('should handle basic delete with join', async function () {
         const query = knex('test_table_two')

diff --git a/test/integration2/query/insert/inserts.spec.js b/test/integration2/query/insert/inserts.spec.js
@@ -914,6 +914,20 @@ describe('Inserts', function () {
           });
       });
 
+      it('#5738 should handle insert with comments', async function () {
+        await knex('test_default_table')
+          .insert({}, 'id')
+          .comment('insert into test_default_table')
+          .testSql(function (tester) {
+            tester(
+              'mysql',
+              '/* insert into test_default_table */ insert into `test_default_table` () values ()',
+              [],
+              [2]
+            );
+          });
+      });
+
       it('should handle empty arrays inserts', async function () {
         await knex.schema.createTable('test_default_table2', function (qb) {
           qb.increments().primary();

diff --git a/test/integration2/query/update/updates.spec.js b/test/integration2/query/update/updates.spec.js
@@ -84,6 +84,25 @@ describe('Updates', function () {
           });
       });
 
+      it('#5738 should handle update with comments', async function () {
+        await knex('accounts')
+          .where('id', 1)
+          .update({
+            first_name: 'User',
+            last_name: 'Test',
+            email: 'test100@example.com',
+          })
+          .comment('update in account')
+          .testSql(function (tester) {
+            tester(
+              'mysql',
+              '/* update in account */ update `accounts` set `first_name` = ?, `last_name` = ?, `email` = ? where `id` = ?',
+              ['User', 'Test', 'test100@example.com', 1],
+              1
+            );
+          });
+      });
+
       it('should allow for null updates', async function () {
         await knex('accounts')
           .where('id', 1000)

diff --git a/test/unit/query/builder.js b/test/unit/query/builder.js
@@ -9508,6 +9508,55 @@ describe('QueryBuilder', () => {
     );
   });
 
+  it('#5738 - should allow query comments in querybuilder for insert statement', () => {
+    testsql(
+      qb()
+        .into('users')
+        .insert({ email: 'foo' })
+        .comment('Added comment 1')
+        .comment('Added comment 2'),
+
+      {
+        mysql: {
+          sql: '/* Added comment 1 */ /* Added comment 2 */ insert into `users` (`email`) values (?)',
+          bindings: ['foo'],
+        },
+      }
+    );
+  });
+
+  it('#5738 - should allow query comments in querybuilder for update statement', () => {
+    testsql(
+      qb()
+        .from('users')
+        .update({ email: 'foo' })
+        .comment('Added comment 1')
+        .comment('Added comment 2'),
+      {
+        mysql: {
+          sql: '/* Added comment 1 */ /* Added comment 2 */ update `users` set `email` = ?',
+          bindings: ['foo'],
+        },
+      }
+    );
+  });
+
+  it('#5738 - should allow query comments in querybuilder for delete statement', () => {
+    testsql(
+      qb()
+        .from('users')
+        .delete()
+        .comment('Added comment 1')
+        .comment('Added comment 2'),
+      {
+        mysql: {
+          sql: '/* Added comment 1 */ /* Added comment 2 */ delete from `users`',
+          bindings: [],
+        },
+      }
+    );
+  });
+
   it('#1982 (2) - should throw error on non string', () => {
     try {
       testsql(qb().from('testtable').comment({ prop: 'val' }), {