fix(oracle): pass duplicated parameters correctly to the client when executing a query (#11537)

ormconfig.sample.json

@@ -44,6 +44,12 @@
     "database": "temp/better-sqlite3db.db",
     "logging": false
   },
+  {
+    "skip": true,
+    "name": "sqljs",
+    "type": "sqljs",
+    "logging": false
+  },
   {
     "skip": false,
     "name": "postgres",

src/driver/oracle/OracleDriver.ts

@@ -386,7 +386,6 @@ export class OracleDriver implements Driver {
         if (!parameters || !Object.keys(parameters).length)
             return [sql, escapedParameters]
 
-        const parameterIndexMap = new Map<string, number>()
         sql = sql.replace(
             /:(\.\.\.)?([A-Za-z0-9_.]+)/g,
             (full, isArray: string, key: string): string => {
@@ -394,10 +393,6 @@ export class OracleDriver implements Driver {
                     return full
                 }
 
-                if (parameterIndexMap.has(key)) {
-                    return this.parametersPrefix + parameterIndexMap.get(key)
-                }
-
                 const value: any = parameters[key]
 
                 if (isArray) {
@@ -421,7 +416,7 @@ export class OracleDriver implements Driver {
                 }
 
                 escapedParameters.push(value)
-                parameterIndexMap.set(key, escapedParameters.length)
+
                 return this.createParameter(key, escapedParameters.length - 1)
             },
         ) // todo: make replace only in value statements, otherwise problems

test/functional/query-builder/parameters/entity/Person.ts

@@ -0,0 +1,13 @@
+import { Column, Entity, PrimaryGeneratedColumn } from "../../../../../src"
+
+@Entity()
+export class Person {
+    @PrimaryGeneratedColumn()
+    id: number
+
+    @Column()
+    firstName: string
+
+    @Column()
+    lastName: string
+}

test/functional/query-builder/parameters/reused-parameters.ts

@@ -0,0 +1,116 @@
+import { expect } from "chai"
+import "reflect-metadata"
+import { DataSource } from "../../../../src/data-source/DataSource"
+import {
+    closeTestingConnections,
+    createTestingConnections,
+    reloadTestingDatabases,
+} from "../../../utils/test-utils"
+import { Person } from "./entity/Person"
+import { DriverUtils } from "../../../../src/driver/DriverUtils"
+
+describe("query builder > parameters > reused parameters", () => {
+    let dataSources: DataSource[]
+    before(
+        async () =>
+            (dataSources = await createTestingConnections({
+                entities: [Person],
+                schemaCreate: true,
+                dropSchema: true,
+            })),
+    )
+
+    beforeEach(() => reloadTestingDatabases(dataSources))
+    after(() => closeTestingConnections(dataSources))
+
+    it("should generate a valid query", () =>
+        Promise.all(
+            dataSources.map(async (dataSource) => {
+                const personRepository = dataSource.getRepository(Person)
+                await personRepository.save([
+                    { firstName: "Jane", lastName: "Smith" },
+                    { firstName: "Johanna", lastName: "Schmidt" },
+                    { firstName: "Ioana", lastName: "Fieraru" },
+                    { firstName: "Giovanna", lastName: "Ferrari" },
+                ])
+
+                const sqlSubstring =
+                    dataSource.driver.options.type === "oracle"
+                        ? "SUBSTR"
+                        : "SUBSTRING"
+                const firstNameTrimmed = `${sqlSubstring}(${dataSource.driver.escape(
+                    "firstName",
+                )}, 1, :charCount)`
+                const lastNameTrimmed = `${sqlSubstring}(${dataSource.driver.escape(
+                    "lastName",
+                )}, 1, :charCount)`
+
+                const queryBuilder = personRepository
+                    .createQueryBuilder()
+                    .select(firstNameTrimmed, "firstName")
+                    .addSelect(lastNameTrimmed, "lastName")
+                    .setParameters({ charCount: 5 })
+
+                const [query, parameters] = queryBuilder.getQueryAndParameters()
+
+                if (DriverUtils.isPostgresFamily(dataSource.driver)) {
+                    expect(query).to.equal(
+                        'SELECT SUBSTRING("firstName", 1, $1) AS "firstName", SUBSTRING("lastName", 1, $1) AS "lastName" FROM "person" "Person"',
+                    )
+                    expect(parameters).to.have.length(1)
+                } else if (DriverUtils.isMySQLFamily(dataSource.driver)) {
+                    expect(query).to.equal(
+                        "SELECT SUBSTRING(`firstName`, 1, ?) AS `firstName`, SUBSTRING(`lastName`, 1, ?) AS `lastName` FROM `person` `Person`",
+                    )
+                    expect(parameters).to.have.length(2)
+                } else if (DriverUtils.isSQLiteFamily(dataSource.driver)) {
+                    expect(query).to.equal(
+                        'SELECT SUBSTRING("firstName", 1, 5) AS "firstName", SUBSTRING("lastName", 1, 5) AS "lastName" FROM "person" "Person"',
+                    )
+                    expect(parameters).to.have.length(0)
+                } else if (dataSource.driver.options.type === "spanner") {
+                    expect(query).to.equal(
+                        'SELECT SUBSTRING("firstName", 1, @param0) AS "firstName", SUBSTRING("lastName", 1, @param0) AS "lastName" FROM "person" "Person"',
+                    )
+                    expect(parameters).to.have.length(1)
+                } else if (dataSource.driver.options.type === "oracle") {
+                    expect(query).to.equal(
+                        'SELECT SUBSTR("firstName", 1, :1) AS "firstName", SUBSTR("lastName", 1, :2) AS "lastName" FROM "person" "Person"',
+                    )
+                    expect(parameters).to.have.length(2)
+                } else if (dataSource.driver.options.type === "mssql") {
+                    expect(query).to.equal(
+                        'SELECT SUBSTRING("firstName", 1, @0) AS "firstName", SUBSTRING("lastName", 1, @0) AS "lastName" FROM "person" "Person"',
+                    )
+                    expect(parameters).to.have.length(1)
+                } else {
+                    // e.g.: SAP
+                    expect(query).to.equal(
+                        'SELECT SUBSTRING("firstName", 1, ?) AS "firstName", SUBSTRING("lastName", 1, ?) AS "lastName" FROM "person" "Person"',
+                    )
+                    expect(parameters).to.have.length(2)
+                }
+
+                const statistics = await queryBuilder.getRawMany<unknown>()
+
+                expect(statistics).to.deep.equal([
+                    {
+                        firstName: "Jane",
+                        lastName: "Smith",
+                    },
+                    {
+                        firstName: "Johan",
+                        lastName: "Schmi",
+                    },
+                    {
+                        firstName: "Ioana",
+                        lastName: "Fiera",
+                    },
+                    {
+                        firstName: "Giova",
+                        lastName: "Ferra",
+                    },
+                ])
+            }),
+        ))
+})

test/functional/query-builder/parameters/sqlite-parameters.ts

@@ -1,14 +1,14 @@
+import { expect } from "chai"
 import "reflect-metadata"
-import { Example } from "./entity/Example"
+import { DataSource } from "../../../../src"
 import {
     closeTestingConnections,
     createTestingConnections,
     reloadTestingDatabases,
 } from "../../../utils/test-utils"
-import { expect } from "chai"
-import { DataSource } from "../../../../src"
+import { Example } from "./entity/Example"
 
-describe("query builder > parameters", () => {
+describe("query builder > parameters > sqlite", () => {
     let connections: DataSource[]
     before(
         async () =>

test/github-issues/7308/entity/weather.ts

@@ -1,10 +0,0 @@
-import { Column, Entity, PrimaryColumn } from "../../../../src"
-
-@Entity()
-export class Weather {
-    @PrimaryColumn()
-    id: string
-
-    @Column({ type: "float" })
-    temperature: number
-}

test/github-issues/7308/issue-7308.ts

@@ -1,70 +0,0 @@
-import "reflect-metadata"
-import {
-    createTestingConnections,
-    closeTestingConnections,
-    reloadTestingDatabases,
-} from "../../utils/test-utils"
-import { DataSource } from "../../../src/data-source/DataSource"
-import { Weather } from "./entity/weather"
-import { expect } from "chai"
-
-describe("github issues > #7308 queryBuilder makes different parameter identifiers for same parameter, causing problems with groupby", () => {
-    describe("Postgres & cockroachdb", () => {
-        let dataSources: DataSource[]
-        before(
-            async () =>
-                (dataSources = await createTestingConnections({
-                    entities: [Weather],
-                    enabledDrivers: [
-                        "postgres",
-                        "cockroachdb",
-                        "spanner",
-                        "mssql",
-                        "oracle",
-                    ],
-                    schemaCreate: true,
-                    dropSchema: true,
-                })),
-        )
-
-        beforeEach(() => reloadTestingDatabases(dataSources))
-        after(() => closeTestingConnections(dataSources))
-
-        it("should not create different parameters identifiers for the same parameter", () =>
-            Promise.all(
-                dataSources.map(async (dataSource) => {
-                    const [query, parameters] = dataSource
-                        .getRepository(Weather)
-                        .createQueryBuilder()
-                        .select("round(temperature, :floatNumber)")
-                        .addSelect("count(*)", "count")
-                        .groupBy("round(temperature, :floatNumber)")
-                        .setParameters({ floatNumber: 2.4 })
-                        .getQueryAndParameters()
-                    query.should.not.be.undefined
-
-                    if (
-                        dataSource.driver.options.type === "postgres" ||
-                        dataSource.driver.options.type === "cockroachdb"
-                    ) {
-                        expect(query).to.equal(
-                            'SELECT round(temperature, $1), count(*) AS "count" FROM "weather" "Weather" GROUP BY round(temperature, $1)',
-                        )
-                    } else if (dataSource.driver.options.type === "spanner") {
-                        expect(query).to.equal(
-                            'SELECT round(temperature, @param0), count(*) AS "count" FROM "weather" "Weather" GROUP BY round(temperature, @param0)',
-                        )
-                    } else if (dataSource.driver.options.type === "oracle") {
-                        expect(query).to.equal(
-                            'SELECT round(temperature, :1), count(*) AS "count" FROM "weather" "Weather" GROUP BY round(temperature, :1)',
-                        )
-                    } else if (dataSource.driver.options.type === "mssql") {
-                        expect(query).to.equal(
-                            'SELECT round(temperature, @0), count(*) AS "count" FROM "weather" "Weather" GROUP BY round(temperature, @0)',
-                        )
-                    }
-                    return parameters.length.should.eql(1)
-                }),
-            ))
-    })
-})