From dd49a254dc475eedfe72378be2670cc6a61aacf1 Mon Sep 17 00:00:00 2001 From: Alexey Pelykh Date: Fri, 26 Jan 2024 07:26:12 +0100 Subject: [PATCH] fix: don't escape indexPredicate (#10618) * fix: don't escape indexPredicate * style: npm run format --- src/driver/sqlite/SqliteQueryRunner.ts | 2 +- src/query-builder/InsertQueryBuilder.ts | 4 +- .../find-options-locking.ts | 28 +++++++++--- test/github-issues/10191/entity/Example.ts | 20 +++++++++ test/github-issues/10191/index-10191.ts | 44 +++++++++++++++++++ test/github-issues/9988/issue-9988.ts | 16 ++++--- test/utils/xfail.ts | 4 +- 7 files changed, 100 insertions(+), 18 deletions(-) create mode 100644 test/github-issues/10191/entity/Example.ts create mode 100644 test/github-issues/10191/index-10191.ts diff --git a/src/driver/sqlite/SqliteQueryRunner.ts b/src/driver/sqlite/SqliteQueryRunner.ts index 5cd720b9aa..822c292def 100644 --- a/src/driver/sqlite/SqliteQueryRunner.ts +++ b/src/driver/sqlite/SqliteQueryRunner.ts @@ -88,7 +88,7 @@ export class SqliteQueryRunner extends AbstractSqliteQueryRunner { } } - const self = this; + const self = this const handler = function (this: any, err: any, rows: any) { if (err && err.toString().indexOf("SQLITE_BUSY:") !== -1) { if ( diff --git a/src/query-builder/InsertQueryBuilder.ts b/src/query-builder/InsertQueryBuilder.ts index 1c3f39e067..1acc3304ac 100644 --- a/src/query-builder/InsertQueryBuilder.ts +++ b/src/query-builder/InsertQueryBuilder.ts @@ -519,9 +519,7 @@ export class InsertQueryBuilder< indexPredicate && DriverUtils.isPostgresFamily(this.connection.driver) ) { - conflictTarget += ` WHERE ( ${this.escape( - indexPredicate, - )} )` + conflictTarget += ` WHERE ( ${indexPredicate} )` } } else if (conflict) { conflictTarget += ` ON CONSTRAINT ${this.escape( diff --git a/test/functional/repository/find-options-locking/find-options-locking.ts b/test/functional/repository/find-options-locking/find-options-locking.ts index 86c91e833e..020c9b2118 100644 --- a/test/functional/repository/find-options-locking/find-options-locking.ts +++ b/test/functional/repository/find-options-locking/find-options-locking.ts @@ -136,7 +136,9 @@ describe("repository > find options > locking", () => { const originalQuery = entityManager.queryRunner!.query.bind( entityManager.queryRunner, ) - entityManager.queryRunner!.query = (...args: Parameters) => { + entityManager.queryRunner!.query = ( + ...args: Parameters + ) => { executedSql.push(args[0]) return originalQuery(...args) } @@ -176,7 +178,9 @@ describe("repository > find options > locking", () => { const originalQuery = entityManager.queryRunner!.query.bind( entityManager.queryRunner, ) - entityManager.queryRunner!.query = (...args: Parameters) => { + entityManager.queryRunner!.query = ( + ...args: Parameters + ) => { executedSql.push(args[0]) return originalQuery(...args) } @@ -205,7 +209,9 @@ describe("repository > find options > locking", () => { const originalQuery = entityManager.queryRunner!.query.bind( entityManager.queryRunner, ) - entityManager.queryRunner!.query = (...args: Parameters) => { + entityManager.queryRunner!.query = ( + ...args: Parameters + ) => { executedSql.push(args[0]) return originalQuery(...args) } @@ -244,7 +250,9 @@ describe("repository > find options > locking", () => { const originalQuery = entityManager.queryRunner!.query.bind( entityManager.queryRunner, ) - entityManager.queryRunner!.query = (...args: Parameters) => { + entityManager.queryRunner!.query = ( + ...args: Parameters + ) => { executedSql.push(args[0]) return originalQuery(...args) } @@ -286,7 +294,9 @@ describe("repository > find options > locking", () => { const originalQuery = entityManager.queryRunner!.query.bind( entityManager.queryRunner, ) - entityManager.queryRunner!.query = (...args: Parameters) => { + entityManager.queryRunner!.query = ( + ...args: Parameters + ) => { executedSql.push(args[0]) return originalQuery(...args) } @@ -323,7 +333,9 @@ describe("repository > find options > locking", () => { const originalQuery = entityManager.queryRunner!.query.bind( entityManager.queryRunner, ) - entityManager.queryRunner!.query = (...args: Parameters) => { + entityManager.queryRunner!.query = ( + ...args: Parameters + ) => { executedSql.push(args[0]) return originalQuery(...args) } @@ -361,7 +373,9 @@ describe("repository > find options > locking", () => { const originalQuery = entityManager.queryRunner!.query.bind( entityManager.queryRunner, ) - entityManager.queryRunner!.query = (...args: Parameters) => { + entityManager.queryRunner!.query = ( + ...args: Parameters + ) => { executedSql.push(args[0]) return originalQuery(...args) } diff --git a/test/github-issues/10191/entity/Example.ts b/test/github-issues/10191/entity/Example.ts new file mode 100644 index 0000000000..155bca399c --- /dev/null +++ b/test/github-issues/10191/entity/Example.ts @@ -0,0 +1,20 @@ +import { Column, Entity, Index, PrimaryGeneratedColumn } from "../../../../src" + +@Entity() +@Index(["nonNullable", "nullable"], { + unique: true, + where: '"nullable" IS NOT NULL', +}) +export class Example { + @PrimaryGeneratedColumn("uuid") + id?: string + + @Column({ type: "text" }) + nonNullable: string + + @Column({ type: "text", nullable: true }) + nullable: string | null + + @Column({ type: "text" }) + value: string +} diff --git a/test/github-issues/10191/index-10191.ts b/test/github-issues/10191/index-10191.ts new file mode 100644 index 0000000000..b3fa9312b5 --- /dev/null +++ b/test/github-issues/10191/index-10191.ts @@ -0,0 +1,44 @@ +import "reflect-metadata" +import { DataSource } from "../../../src" +import { + closeTestingConnections, + createTestingConnections, + reloadTestingDatabases, +} from "../../utils/test-utils" +import { Example } from "./entity/Example" + +describe("github issues > #10191 incorrect escaping of indexPredicate", () => { + let connections: DataSource[] + + before( + async () => + (connections = await createTestingConnections({ + entities: [Example], + enabledDrivers: ["postgres"], + })), + ) + beforeEach(() => reloadTestingDatabases(connections)) + after(() => closeTestingConnections(connections)) + + it("should not fail", () => + Promise.all( + connections.map(async (connection) => { + await connection.manager.upsert( + Example, + { + nonNullable: "nonNullable", + nullable: "nullable", + value: "value", + }, + { + conflictPaths: { + nonNullable: true, + nullable: true, + }, + skipUpdateIfNoValuesChanged: true, + indexPredicate: '"nullable" IS NOT NULL', + }, + ) + }), + )) +}) diff --git a/test/github-issues/9988/issue-9988.ts b/test/github-issues/9988/issue-9988.ts index 7da0891d3e..d0d0348256 100644 --- a/test/github-issues/9988/issue-9988.ts +++ b/test/github-issues/9988/issue-9988.ts @@ -39,11 +39,12 @@ describe("github issues > #9988 RelationIdLoader reuses the same queryplanner wi const productTwoId = 2 await categoryRepo.save(categoryOne) await categoryRepo.save(categoryTwo) - const options = (id: number) => ({ - relationLoadStrategy: "query", - where: { id }, - relations: { categories: true }, - } as FindManyOptions) + const options = (id: number) => + ({ + relationLoadStrategy: "query", + where: { id }, + relations: { categories: true }, + } as FindManyOptions) // Create a custom repository that uses a query builder without query planner // For both methods, relationLoadStrategy is set to "query", where the bug lies. @@ -84,7 +85,10 @@ describe("github issues > #9988 RelationIdLoader reuses the same queryplanner wi txnManager.withRepository(productRepo) const product = customProductRepo.create({ id: productTwoId, - categories: [{ id: categoryOne.id }, { id: categoryTwo.id }], + categories: [ + { id: categoryOne.id }, + { id: categoryTwo.id }, + ], }) await customProductRepo.save(product) diff --git a/test/utils/xfail.ts b/test/utils/xfail.ts index 30ad49a0d2..87cf936208 100644 --- a/test/utils/xfail.ts +++ b/test/utils/xfail.ts @@ -21,7 +21,9 @@ const wrap = ( return new Promise((ok, fail) => { if (fn.length > 1) { - (fn as Func).call(context as unknown as Context, (err: any) => (err ? fail(err) : ok())) + ;(fn as Func).call(context as unknown as Context, (err: any) => + err ? fail(err) : ok(), + ) } else { ok((fn as AsyncFunc).call(context as unknown as Context)) }