fix: resolve issue queryBuilder makes different parameter identifiers for same parameter (#10327)

Closes: #7308
This commit is contained in:
Fawzi Abdulfattah 2023-12-29 13:19:55 +02:00 committed by GitHub
parent bfc1cc5ab4
commit 6c918ea392
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 146 additions and 6 deletions

View File

@ -112,6 +112,11 @@ export interface Driver {
*/
mappedDataTypes: MappedColumnTypes
/**
* The prefix used for the parameters
*/
parametersPrefix?: string
/**
* Max length allowed by the DBMS for aliases (execution of queries).
*/

View File

@ -224,6 +224,11 @@ export class CockroachDriver implements Driver {
metadataValue: "string",
}
/**
* The prefix used for the parameters
*/
parametersPrefix: string = "$"
/**
* Default values of length, precision and scale depends on column data type.
* Used in the cases when length/precision/scale is not specified by user.
@ -509,6 +514,7 @@ export class CockroachDriver 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 => {
@ -516,6 +522,10 @@ export class CockroachDriver implements Driver {
return full
}
if (parameterIndexMap.has(key)) {
return this.parametersPrefix + parameterIndexMap.get(key)
}
let value: any = parameters[key]
if (isArray) {
@ -535,6 +545,7 @@ export class CockroachDriver 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
@ -961,7 +972,7 @@ export class CockroachDriver implements Driver {
* Creates an escaped parameter.
*/
createParameter(parameterName: string, index: number): string {
return "$" + (index + 1)
return this.parametersPrefix + (index + 1)
}
// -------------------------------------------------------------------------

View File

@ -215,6 +215,11 @@ export class OracleDriver implements Driver {
metadataValue: "clob",
}
/**
* The prefix used for the parameters
*/
parametersPrefix: string = ":"
/**
* Default values of length, precision and scale depends on column data type.
* Used in the cases when length/precision/scale is not specified by user.
@ -378,6 +383,7 @@ 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 => {
@ -385,6 +391,10 @@ export class OracleDriver implements Driver {
return full
}
if (parameterIndexMap.has(key)) {
return this.parametersPrefix + parameterIndexMap.get(key)
}
let value: any = parameters[key]
if (isArray) {
@ -408,6 +418,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
@ -928,7 +939,7 @@ export class OracleDriver implements Driver {
* Creates an escaped parameter.
*/
createParameter(parameterName: string, index: number): string {
return ":" + (index + 1)
return this.parametersPrefix + (index + 1)
}
/**

View File

@ -257,6 +257,11 @@ export class PostgresDriver implements Driver {
metadataValue: "text",
}
/**
* The prefix used for the parameters
*/
parametersPrefix: string = "$"
/**
* Default values of length, precision and scale depends on column data type.
* Used in the cases when length/precision/scale is not specified by user.
@ -830,6 +835,7 @@ export class PostgresDriver 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 => {
@ -837,6 +843,10 @@ export class PostgresDriver implements Driver {
return full
}
if (parameterIndexMap.has(key)) {
return this.parametersPrefix + parameterIndexMap.get(key)
}
let value: any = parameters[key]
if (isArray) {
@ -856,6 +866,7 @@ export class PostgresDriver 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
@ -1399,7 +1410,7 @@ export class PostgresDriver implements Driver {
* Creates an escaped parameter.
*/
createParameter(parameterName: string, index: number): string {
return "$" + (index + 1)
return this.parametersPrefix + (index + 1)
}
// -------------------------------------------------------------------------

View File

@ -157,6 +157,11 @@ export class SpannerDriver implements Driver {
metadataValue: "string",
}
/**
* The prefix used for the parameters
*/
parametersPrefix: string = "@param"
/**
* Default values of length, precision and scale depends on column data type.
* Used in the cases when length/precision/scale is not specified by user.
@ -251,6 +256,7 @@ export class SpannerDriver 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 => {
@ -258,6 +264,10 @@ export class SpannerDriver implements Driver {
return full
}
if (parameterIndexMap.has(key)) {
return this.parametersPrefix + parameterIndexMap.get(key)
}
let value: any = parameters[key]
if (value === null) {
@ -279,7 +289,9 @@ export class SpannerDriver implements Driver {
if (value instanceof Function) {
return value()
}
escapedParameters.push(value)
parameterIndexMap.set(key, escapedParameters.length - 1)
return this.createParameter(key, escapedParameters.length - 1)
},
) // todo: make replace only in value statements, otherwise problems
@ -717,7 +729,7 @@ export class SpannerDriver implements Driver {
* Creates an escaped parameter.
*/
createParameter(parameterName: string, index: number): string {
return "@param" + index
return this.parametersPrefix + index
}
// -------------------------------------------------------------------------

View File

@ -211,6 +211,11 @@ export class SqlServerDriver implements Driver {
metadataValue: "nvarchar(MAX)" as any,
}
/**
* The prefix used for the parameters
*/
parametersPrefix: string = "@"
/**
* Default values of length, precision and scale depends on column data type.
* Used in the cases when length/precision/scale is not specified by user.
@ -371,6 +376,7 @@ export class SqlServerDriver 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 => {
@ -378,6 +384,10 @@ export class SqlServerDriver implements Driver {
return full
}
if (parameterIndexMap.has(key)) {
return this.parametersPrefix + parameterIndexMap.get(key)
}
let value: any = parameters[key]
if (isArray) {
@ -397,6 +407,7 @@ export class SqlServerDriver implements Driver {
}
escapedParameters.push(value)
parameterIndexMap.set(key, escapedParameters.length - 1)
return this.createParameter(key, escapedParameters.length - 1)
},
) // todo: make replace only in value statements, otherwise problems
@ -908,7 +919,7 @@ export class SqlServerDriver implements Driver {
* Creates an escaped parameter.
*/
createParameter(parameterName: string, index: number): string {
return "@" + index
return this.parametersPrefix + index
}
// -------------------------------------------------------------------------

View File

@ -10,7 +10,6 @@ import { LessThan } from "../../../../src"
import { expect } from "chai"
describe("repository > aggregate methods", () => {
debugger
let connections: DataSource[]
let repository: Repository<Post>

View File

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

View File

@ -0,0 +1,70 @@
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)
}),
))
})
})