Skip to content

Commit

Permalink
fix: resolve issue queryBuilder makes different parameter identifiers…
Browse files Browse the repository at this point in the history
… for same parameter (#10327)

Closes: #7308
  • Loading branch information
iifawzi committed Dec 29, 2023
1 parent bfc1cc5 commit 6c918ea
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 6 deletions.
5 changes: 5 additions & 0 deletions src/driver/Driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).
*/
Expand Down
13 changes: 12 additions & 1 deletion src/driver/cockroachdb/CockroachDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -509,13 +514,18 @@ 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 => {
if (!parameters.hasOwnProperty(key)) {
return full
}

if (parameterIndexMap.has(key)) {
return this.parametersPrefix + parameterIndexMap.get(key)
}

let value: any = parameters[key]

if (isArray) {
Expand All @@ -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
Expand Down Expand Up @@ -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)
}

// -------------------------------------------------------------------------
Expand Down
13 changes: 12 additions & 1 deletion src/driver/oracle/OracleDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -378,13 +383,18 @@ 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 => {
if (!parameters.hasOwnProperty(key)) {
return full
}

if (parameterIndexMap.has(key)) {
return this.parametersPrefix + parameterIndexMap.get(key)
}

let value: any = parameters[key]

if (isArray) {
Expand All @@ -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
Expand Down Expand Up @@ -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)
}

/**
Expand Down
13 changes: 12 additions & 1 deletion src/driver/postgres/PostgresDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -830,13 +835,18 @@ 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 => {
if (!parameters.hasOwnProperty(key)) {
return full
}

if (parameterIndexMap.has(key)) {
return this.parametersPrefix + parameterIndexMap.get(key)
}

let value: any = parameters[key]

if (isArray) {
Expand All @@ -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
Expand Down Expand Up @@ -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)
}

// -------------------------------------------------------------------------
Expand Down
14 changes: 13 additions & 1 deletion src/driver/spanner/SpannerDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -251,13 +256,18 @@ 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 => {
if (!parameters.hasOwnProperty(key)) {
return full
}

if (parameterIndexMap.has(key)) {
return this.parametersPrefix + parameterIndexMap.get(key)
}

let value: any = parameters[key]

if (value === null) {
Expand All @@ -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
Expand Down Expand Up @@ -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
}

// -------------------------------------------------------------------------
Expand Down
13 changes: 12 additions & 1 deletion src/driver/sqlserver/SqlServerDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -371,13 +376,18 @@ 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 => {
if (!parameters.hasOwnProperty(key)) {
return full
}

if (parameterIndexMap.has(key)) {
return this.parametersPrefix + parameterIndexMap.get(key)
}

let value: any = parameters[key]

if (isArray) {
Expand All @@ -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
Expand Down Expand Up @@ -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
}

// -------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { LessThan } from "../../../../src"
import { expect } from "chai"

describe("repository > aggregate methods", () => {
debugger
let connections: DataSource[]
let repository: Repository<Post>

Expand Down
10 changes: 10 additions & 0 deletions test/github-issues/7308/entity/weather.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Column, Entity, PrimaryColumn } from "../../../../src"

@Entity()
export class Weather {
@PrimaryColumn()
id: string

@Column({ type: "float" })
temperature: number
}
70 changes: 70 additions & 0 deletions test/github-issues/7308/issue-7308.ts
Original file line number Diff line number Diff line change
@@ -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)
}),
))
})
})

0 comments on commit 6c918ea

Please sign in to comment.