Add ability to prepend query comments (#5289)

Co-authored-by: Ryan Fink <ryanjfink@gmail.com>

lib/dialects/mssql/query/mssql-querycompiler.js

@@ -11,6 +11,7 @@ const {
 } = require('../../../formatter/wrappingFormatter');
 
 const components = [
+  'comments',
   'columns',
   'join',
   'lock',

lib/dialects/oracle/query/oracle-querycompiler.js

@@ -12,6 +12,7 @@ const { ReturningHelper } = require('../utils');
 const { isString } = require('../../../util/is');
 
 const components = [
+  'comments',
   'columns',
   'join',
   'where',

lib/query/querybuilder.js

@@ -61,6 +61,7 @@ class Builder extends EventEmitter {
     this.client = client;
     this.and = this;
     this._single = {};
+    this._comments = [];
     this._statements = [];
     this._method = 'select';
     if (client.config) {
@@ -88,6 +89,7 @@ class Builder extends EventEmitter {
     const cloned = new this.constructor(this.client);
     cloned._method = this._method;
     cloned._single = clone(this._single);
+    cloned._comments = clone(this._comments);
     cloned._statements = clone(this._statements);
     cloned._debug = this._debug;
 
@@ -235,6 +237,21 @@ class Builder extends EventEmitter {
     return this;
   }
 
+  // Adds a comment to the query
+  comment(txt) {
+    if (!isString(txt)) {
+      throw new Error('Comment must be a string');
+    }
+    const forbiddenChars = ['/*', '*/', '?'];
+    if (forbiddenChars.some((chars) => txt.includes(chars))) {
+      throw new Error(`Cannot include ${forbiddenChars.join(', ')} in comment`);
+    }
+    this._comments.push({
+      comment: txt,
+    });
+    return this;
+  }
+
   // Allow for a sub-select to be explicitly aliased as a column,
   // without needing to compile the query in a where.
   as(column) {

lib/query/querycompiler.js

@@ -28,6 +28,7 @@ const {
 const debugBindings = debug('knex:bindings');
 
 const components = [
+  'comments',
   'columns',
   'join',
   'where',
@@ -50,6 +51,7 @@ class QueryCompiler {
     this.method = builder._method || 'select';
     this.options = builder._options;
     this.single = builder._single;
+    this.queryComments = builder._comments;
     this.timeout = builder._timeout || false;
     this.cancelOnTimeout = builder._cancelOnTimeout || false;
     this.grouped = groupBy(builder._statements, 'grouping');
@@ -138,6 +140,7 @@ class QueryCompiler {
         case 'union':
           unionStatement = statement;
           break;
+        case 'comments':
         case 'columns':
         case 'join':
         case 'where':
@@ -317,6 +320,14 @@ class QueryCompiler {
     );
   }
 
+  // Add comments to the query
+  comments() {
+    if (!this.queryComments.length) return '';
+    return this.queryComments
+      .map((comment) => `/* ${comment.comment} */`)
+      .join(' ');
+  }
+
   _aggregate(stmt, { aliasSeparator = ' as ', distinctParentheses } = {}) {
     const value = stmt.value;
     const method = stmt.method;

test/integration2/query/select/selects.spec.js

@@ -863,6 +863,16 @@ describe('Selects', function () {
         });
       });
 
+      it('#1982 - Allow comments in db', async function () {
+        await knex('accounts')
+          .comment('Integration Comment')
+          .select('first_name', 'email')
+          .limit(1)
+          .then(function(results) {
+            expect(results).to.have.length(1);
+          });
+      });
+
       describe('recursive CTE support', function () {
         before(async function () {
           await knex.schema.dropTableIfExists('rcte');

test/unit/query/builder.js

@@ -9440,6 +9440,156 @@ describe('QueryBuilder', () => {
     );
   });
 
+  it('#1982 - should allow query comments in querybuilder', () => {
+    testsql(
+      qb()
+        .from('testtable')
+        .comment('Added comment 1')
+        .comment('Added comment 2'),
+      {
+        mysql: {
+          sql:
+            '/* Added comment 1 */ /* Added comment 2 */ select * from `testtable`',
+          bindings: [],
+        },
+        oracledb: {
+          sql:
+            '/* Added comment 1 */ /* Added comment 2 */ select * from "testtable"',
+          bindings: [],
+        },
+        mssql: {
+          sql:
+            '/* Added comment 1 */ /* Added comment 2 */ select * from [testtable]',
+          bindings: [],
+        },
+        pg: {
+          sql:
+            '/* Added comment 1 */ /* Added comment 2 */ select * from "testtable"',
+          bindings: [],
+        },
+        'pg-redshift': {
+          sql:
+            '/* Added comment 1 */ /* Added comment 2 */ select * from "testtable"',
+          bindings: [],
+        },
+      }
+    );
+  });
+
+  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: [],
+          },
+        }
+      );
+      expect(true).to.equal(
+        false,
+        'Expected to throw error in compilation about non-string'
+      );
+    } catch (error) {
+      expect(error.message).to.contain('Comment must be a string');
+    }
+  });
+
+  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: [],
+          },
+        }
+      );
+      expect(true).to.equal(
+        false,
+        'Expected to throw error in compilation about non-string'
+      );
+    } catch (error) {
+      expect(error.message).to.contain('Cannot include /*, */, ? in comment');
+    }
+  });
+
+  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: [],
+          },
+        }
+      );
+      expect(true).to.equal(
+        false,
+        'Expected to throw error in compilation about non-string'
+      );
+    } catch (error) {
+      expect(error.message).to.contain('Cannot include /*, */, ? in comment');
+    }
+  });
+
   it('#4199 - allows hint comments in subqueries', () => {
     testsql(
       qb()

types/index.d.ts

@@ -597,6 +597,7 @@ export declare namespace Knex {
     as: As<TRecord, TResult>;
     columns: Select<TRecord, TResult>;
     column: Select<TRecord, TResult>;
+    comment: Comment<TRecord, TResult>;
     hintComment: HintComment<TRecord, TResult>;
     from: Table<TRecord, TResult>;
     fromRaw: Table<TRecord, TResult>;
@@ -1359,6 +1360,10 @@ export declare namespace Knex {
     ): QueryBuilder<TRecord, TResult>;
   }
 
+  interface Comment<TRecord extends {} = any, TResult = any> {
+    (comment: string): QueryBuilder<TRecord, TResult>;
+  }
+
   interface HintComment<TRecord extends {} = any, TResult = any> {
     (hint: string): QueryBuilder<TRecord, TResult>;
     (hints: readonly string[]): QueryBuilder<TRecord, TResult>;