Skip to content

Commit 63a3b9a

Browse files
fix: multiple relations with same column name(s) generate invalid SELECT statement (#11400)
* fix: Multiple relations with same columns cause invalid SQL to be generated Closes: #1668, #9788, #9814, #10121, #10148, #11109, #11132, #11180 * refactor: extract cloneObject util * fix: improve cloneObject * test: remove duplicate tests * test: transformed the test: add City, Country, and Order entities with composite foreign key relations, * test: change to composite primary key --------- Co-authored-by: Lucian Mocanu <[email protected]>
1 parent af9ecc0 commit 63a3b9a

File tree

7 files changed

+211
-0
lines changed

7 files changed

+211
-0
lines changed

src/metadata-builder/RelationJoinColumnBuilder.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { JoinColumnMetadataArgs } from "../metadata-args/JoinColumnMetadataArgs"
66
import { DataSource } from "../data-source/DataSource"
77
import { TypeORMError } from "../error"
88
import { DriverUtils } from "../driver/DriverUtils"
9+
import { OrmUtils } from "../util/OrmUtils"
910

1011
/**
1112
* Builds join column for the many-to-one and one-to-one owner relations.
@@ -233,6 +234,11 @@ export class RelationJoinColumnBuilder {
233234
},
234235
})
235236
relation.entityMetadata.registerColumn(relationalColumn)
237+
} else if (relationalColumn.referencedColumn) {
238+
// Clone the relational column to prevent modifying the original when multiple
239+
// relations reference the same column. This ensures each relation gets its own
240+
// copy with independent referencedColumn and type properties.
241+
relationalColumn = OrmUtils.cloneObject(relationalColumn)
236242
}
237243
relationalColumn.referencedColumn = referencedColumn // its important to set it here because we need to set referenced column for user defined join column
238244
relationalColumn.type = referencedColumn.type // also since types of relational column and join column must be equal we override user defined column type

src/util/OrmUtils.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,20 @@ export class OrmUtils {
9292
return target
9393
}
9494

95+
/**
96+
* Creates a shallow copy of the object, without invoking the constructor
97+
*/
98+
public static cloneObject<T extends object>(object: T): T {
99+
if (object === null || object === undefined) {
100+
return object
101+
}
102+
103+
return Object.assign(
104+
Object.create(Object.getPrototypeOf(object)),
105+
object,
106+
)
107+
}
108+
95109
/**
96110
* Deep compare objects.
97111
*
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { Column, Entity, PrimaryColumn } from "../../../../../src"
2+
3+
@Entity()
4+
export class City {
5+
@PrimaryColumn()
6+
name: string
7+
8+
@PrimaryColumn()
9+
countryName: string
10+
11+
@Column()
12+
population: number
13+
}
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import {
2+
Column,
3+
Entity,
4+
JoinColumn,
5+
ManyToOne,
6+
PrimaryColumn,
7+
} from "../../../../../src"
8+
import { City } from "./City"
9+
import { Country } from "./Country"
10+
11+
@Entity()
12+
export class Company {
13+
@PrimaryColumn()
14+
name: string
15+
16+
@Column()
17+
countryName: string
18+
19+
@ManyToOne(() => Country)
20+
@JoinColumn({ name: "countryName" })
21+
country?: Country
22+
23+
@Column()
24+
cityName: string
25+
26+
@ManyToOne(() => City)
27+
@JoinColumn([
28+
{ name: "cityName", referencedColumnName: "name" },
29+
{ name: "countryName", referencedColumnName: "countryName" },
30+
])
31+
city?: City
32+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { Column, Entity, PrimaryColumn } from "../../../../../src"
2+
3+
@Entity()
4+
export class Country {
5+
@PrimaryColumn()
6+
name: string
7+
8+
@Column()
9+
region: string
10+
}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { expect } from "chai"
2+
3+
import { DataSource } from "../../../../src"
4+
5+
import {
6+
closeTestingConnections,
7+
createTestingConnections,
8+
reloadTestingDatabases,
9+
} from "../../../utils/test-utils"
10+
import { City } from "./entity/City"
11+
import { Country } from "./entity/Country"
12+
import { Company } from "./entity/Company"
13+
14+
describe("metadata builder > RelationJoinColumnBuilder", () => {
15+
let dataSources: DataSource[]
16+
17+
before(
18+
async () =>
19+
(dataSources = await createTestingConnections({
20+
entities: [__dirname + "/entity/*{.js,.ts}"],
21+
})),
22+
)
23+
beforeEach(() => reloadTestingDatabases(dataSources))
24+
after(() => closeTestingConnections(dataSources))
25+
26+
it("should not throw error when loading entities with composite FK with shared columns", () =>
27+
Promise.all(
28+
dataSources.map(async (dataSource) => {
29+
await dataSource.getRepository(Country).save([
30+
{ name: "Texas", region: "USA" },
31+
{ name: "France", region: "EU" },
32+
] satisfies Country[])
33+
34+
await dataSource.getRepository(City).save([
35+
{
36+
name: "Paris",
37+
countryName: "France",
38+
population: 2_100_000,
39+
},
40+
{
41+
name: "Paris",
42+
countryName: "Texas",
43+
population: 25_000,
44+
},
45+
{
46+
name: "Strasbourg",
47+
countryName: "France",
48+
population: 270_000,
49+
},
50+
{
51+
name: "Lyon",
52+
countryName: "France",
53+
population: 720_000,
54+
},
55+
{
56+
name: "Houston",
57+
countryName: "Texas",
58+
population: 2_300_000,
59+
},
60+
] satisfies City[])
61+
62+
await dataSource.getRepository(Company).save([
63+
{ name: "NASA", countryName: "Texas", cityName: "Houston" },
64+
{ name: "AXA", countryName: "France", cityName: "Paris" },
65+
] satisfies Company[])
66+
67+
const companies = await dataSource.getRepository(Company).find({
68+
relations: { city: true, country: true },
69+
order: { name: "asc" },
70+
})
71+
72+
expect(companies).to.deep.members([
73+
{
74+
name: "AXA",
75+
countryName: "France",
76+
cityName: "Paris",
77+
city: {
78+
countryName: "France",
79+
name: "Paris",
80+
population: 2_100_000,
81+
},
82+
country: { name: "France", region: "EU" },
83+
},
84+
{
85+
name: "NASA",
86+
countryName: "Texas",
87+
cityName: "Houston",
88+
city: {
89+
countryName: "Texas",
90+
name: "Houston",
91+
population: 2_300_000,
92+
},
93+
country: { name: "Texas", region: "USA" },
94+
},
95+
] satisfies Company[])
96+
}),
97+
))
98+
})

test/unit/util/orm-utils.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,42 @@ describe(`OrmUtils`, () => {
160160
expect(result.foo).to.equal(foo)
161161
})
162162
})
163+
164+
describe("cloneObject", () => {
165+
it("should create a shallow copy of an instance without invoking the constructor", () => {
166+
class SomeClass {
167+
static hasConstructorBeenInvoked = false
168+
169+
constructor(
170+
public someString: string,
171+
public someNumber: number,
172+
) {
173+
if (SomeClass.hasConstructorBeenInvoked) {
174+
throw Error(
175+
"The constructor was invoked a second time!",
176+
)
177+
}
178+
SomeClass.hasConstructorBeenInvoked = true
179+
}
180+
181+
clone() {
182+
return new SomeClass(this.someString, this.someNumber)
183+
}
184+
}
185+
186+
const obj = new SomeClass("string", 0)
187+
188+
let objCopy: SomeClass | undefined
189+
let objCopy2: SomeClass | undefined
190+
expect(() => {
191+
objCopy = OrmUtils.cloneObject(obj)
192+
}).not.to.throw()
193+
expect(() => {
194+
objCopy2 = obj.clone()
195+
}).to.throw()
196+
expect(objCopy).not.to.equal(obj)
197+
expect(objCopy).to.deep.equal(obj)
198+
expect(objCopy2).to.equal(undefined)
199+
})
200+
})
163201
})

0 commit comments

Comments
 (0)