From 579ea042b4afaf536ec4701c67ec9b6dc1c712f7 Mon Sep 17 00:00:00 2001 From: James Ward Date: Thu, 15 Oct 2020 04:15:44 -0400 Subject: [PATCH] feat: support query comments in the query builder (#6892) add a `comment` method to the QueryBuilder so we can include an arbitrary comment in our queries for a variety of purposes - from query plan stability to SQL reverse proxy routing, to debugging. the comment builder is supported in selects, inserts, deletes, and updates this is directly inspired by the functionality supported by hibernate to handle SQL Query comments it uses C-style queries which are ANSI SQL 2003 & supported in all of the dialects of SQL that we support as drivers fixes #3643 --- src/query-builder/DeleteQueryBuilder.ts | 3 +- src/query-builder/InsertQueryBuilder.ts | 3 +- src/query-builder/QueryBuilder.ts | 23 +++++ src/query-builder/QueryExpressionMap.ts | 5 ++ src/query-builder/SelectQueryBuilder.ts | 3 +- src/query-builder/UpdateQueryBuilder.ts | 3 +- .../query-builder/comment/entity/Test.ts | 10 +++ .../comment/query-builder-comment.ts | 85 +++++++++++++++++++ 8 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 test/functional/query-builder/comment/entity/Test.ts create mode 100644 test/functional/query-builder/comment/query-builder-comment.ts diff --git a/src/query-builder/DeleteQueryBuilder.ts b/src/query-builder/DeleteQueryBuilder.ts index c4c1358d163..1840a487280 100644 --- a/src/query-builder/DeleteQueryBuilder.ts +++ b/src/query-builder/DeleteQueryBuilder.ts @@ -40,7 +40,8 @@ export class DeleteQueryBuilder extends QueryBuilder implements * Gets generated sql query without parameters being replaced. */ getQuery(): string { - let sql = this.createDeleteExpression(); + let sql = this.createComment(); + sql += this.createDeleteExpression(); return sql.trim(); } diff --git a/src/query-builder/InsertQueryBuilder.ts b/src/query-builder/InsertQueryBuilder.ts index 8192293f627..4d63755fcd4 100644 --- a/src/query-builder/InsertQueryBuilder.ts +++ b/src/query-builder/InsertQueryBuilder.ts @@ -33,7 +33,8 @@ export class InsertQueryBuilder extends QueryBuilder { * Gets generated sql query without parameters being replaced. */ getQuery(): string { - let sql = this.createInsertExpression(); + let sql = this.createComment(); + sql += this.createInsertExpression(); return sql.trim(); } diff --git a/src/query-builder/QueryBuilder.ts b/src/query-builder/QueryBuilder.ts index 1b1d8ef0d56..7d8ddc8727c 100644 --- a/src/query-builder/QueryBuilder.ts +++ b/src/query-builder/QueryBuilder.ts @@ -446,6 +446,16 @@ export abstract class QueryBuilder { return new (this.constructor as any)(this); } + /** + * Includes a Query comment in the query builder. This is helpful for debugging purposes, + * such as finding a specific query in the database server's logs, or for categorization using + * an APM product. + */ + comment(comment: string): this { + this.expressionMap.comment = comment; + return this; + } + /** * Disables escaping. */ @@ -613,6 +623,19 @@ export abstract class QueryBuilder { return statement; } + protected createComment(): string { + if (!this.expressionMap.comment) { + return ""; + } + + // ANSI SQL 2003 support C style comments - comments that start with `/*` and end with `*/` + // In some dialects query nesting is available - but not all. Because of this, we'll need + // to scrub "ending" characters from the SQL but otherwise we can leave everything else + // as-is and it should be valid. + + return `/* ${this.expressionMap.comment.replace("*/", "")} */ `; + } + /** * Creates "WHERE" expression. */ diff --git a/src/query-builder/QueryExpressionMap.ts b/src/query-builder/QueryExpressionMap.ts index e4c638888a9..42f1f61642b 100644 --- a/src/query-builder/QueryExpressionMap.ts +++ b/src/query-builder/QueryExpressionMap.ts @@ -271,6 +271,11 @@ export class QueryExpressionMap { */ nativeParameters: ObjectLiteral = {}; + /** + * Query Comment to include extra information for debugging or other purposes. + */ + comment?: string; + // ------------------------------------------------------------------------- // Constructor // ------------------------------------------------------------------------- diff --git a/src/query-builder/SelectQueryBuilder.ts b/src/query-builder/SelectQueryBuilder.ts index 5fa2fde9e6c..7f95fd7d611 100644 --- a/src/query-builder/SelectQueryBuilder.ts +++ b/src/query-builder/SelectQueryBuilder.ts @@ -51,7 +51,8 @@ export class SelectQueryBuilder extends QueryBuilder implements * Gets generated sql query without parameters being replaced. */ getQuery(): string { - let sql = this.createSelectExpression(); + let sql = this.createComment(); + sql += this.createSelectExpression(); sql += this.createJoinExpression(); sql += this.createWhereExpression(); sql += this.createGroupByExpression(); diff --git a/src/query-builder/UpdateQueryBuilder.ts b/src/query-builder/UpdateQueryBuilder.ts index 2e0497f1f50..19a091da615 100644 --- a/src/query-builder/UpdateQueryBuilder.ts +++ b/src/query-builder/UpdateQueryBuilder.ts @@ -48,7 +48,8 @@ export class UpdateQueryBuilder extends QueryBuilder implements * Gets generated sql query without parameters being replaced. */ getQuery(): string { - let sql = this.createUpdateExpression(); + let sql = this.createComment(); + sql += this.createUpdateExpression(); sql += this.createOrderByExpression(); sql += this.createLimitExpression(); return sql.trim(); diff --git a/test/functional/query-builder/comment/entity/Test.ts b/test/functional/query-builder/comment/entity/Test.ts new file mode 100644 index 00000000000..6312daa316c --- /dev/null +++ b/test/functional/query-builder/comment/entity/Test.ts @@ -0,0 +1,10 @@ +import {Entity} from "../../../../../src/decorator/entity/Entity"; +import {PrimaryGeneratedColumn} from "../../../../../src/decorator/columns/PrimaryGeneratedColumn"; + +@Entity() +export class Test { + + @PrimaryGeneratedColumn() + id: number; + +} diff --git a/test/functional/query-builder/comment/query-builder-comment.ts b/test/functional/query-builder/comment/query-builder-comment.ts new file mode 100644 index 00000000000..a691251705a --- /dev/null +++ b/test/functional/query-builder/comment/query-builder-comment.ts @@ -0,0 +1,85 @@ +import "reflect-metadata"; +import {closeTestingConnections, createTestingConnections, reloadTestingDatabases} from "../../../utils/test-utils"; +import {Connection} from "../../../../src/connection/Connection"; +import {Test} from "./entity/Test"; +import {expect} from "chai"; + +describe("query builder > comment", () => { + + let connections: Connection[]; + before(async () => connections = await createTestingConnections({ + entities: [Test], + })); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("should scrub end comment pattern from string", () => Promise.all(connections.map(async connection => { + const sql = connection.manager.createQueryBuilder(Test, "test") + .comment("Hello World */") + .getSql(); + + expect(sql).to.match(/^\/\* Hello World \*\/ SELECT/); + }))); + + it("should not allow an empty comment", () => Promise.all(connections.map(async connection => { + const sql = connection.manager.createQueryBuilder(Test, "test") + .comment("") + .getSql(); + + expect(sql).to.not.match(/^\/\* Hello World \*\/ SELECT/); + }))); + + it("should allow a comment with just whitespaces", () => Promise.all(connections.map(async connection => { + const sql = connection.manager.createQueryBuilder(Test, "test") + .comment(" ") + .getSql(); + + expect(sql).to.match(/^\/\* \*\/ SELECT/); + }))); + + it("should allow a multi-line comment", () => Promise.all(connections.map(async connection => { + const sql = connection.manager.createQueryBuilder(Test, "test") + .comment("Hello World\nIt's a beautiful day!") + .getSql(); + + expect(sql).to.match(/^\/\* Hello World\nIt's a beautiful day! \*\/ SELECT/); + }))); + + it("should include comment in select", () => Promise.all(connections.map(async connection => { + const sql = connection.manager.createQueryBuilder(Test, "test") + .comment("Hello World") + .getSql(); + + expect(sql).to.match(/^\/\* Hello World \*\/ SELECT/); + }))); + + it("should include comment in update", () => Promise.all(connections.map(async connection => { + const sql = connection.manager.createQueryBuilder(Test, "test") + .update() + .set({ id: 2 }) + .comment("Hello World") + .getSql(); + + expect(sql).to.match(/^\/\* Hello World \*\/ UPDATE/); + }))); + + it("should include comment in insert", () => Promise.all(connections.map(async connection => { + const sql = connection.manager.createQueryBuilder(Test, "test") + .insert() + .values({ id: 1 }) + .comment("Hello World") + .getSql(); + + expect(sql).to.match(/^\/\* Hello World \*\/ INSERT/); + }))); + + it("should include comment in delete", () => Promise.all(connections.map(async connection => { + const sql = connection.manager.createQueryBuilder(Test, "test") + .delete() + .comment("Hello World") + .getSql(); + + expect(sql).to.match(/^\/\* Hello World \*\/ DELETE/); + }))); + +});