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 <alumni@users.noreply.github.com>
This commit is contained in:
Yevhen Komarov 2025-06-17 19:38:16 +03:00 committed by GitHub
parent af9ecc09cc
commit 63a3b9abc1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 211 additions and 0 deletions

View File

@ -6,6 +6,7 @@ import { JoinColumnMetadataArgs } from "../metadata-args/JoinColumnMetadataArgs"
import { DataSource } from "../data-source/DataSource"
import { TypeORMError } from "../error"
import { DriverUtils } from "../driver/DriverUtils"
import { OrmUtils } from "../util/OrmUtils"
/**
* Builds join column for the many-to-one and one-to-one owner relations.
@ -233,6 +234,11 @@ export class RelationJoinColumnBuilder {
},
})
relation.entityMetadata.registerColumn(relationalColumn)
} else if (relationalColumn.referencedColumn) {
// Clone the relational column to prevent modifying the original when multiple
// relations reference the same column. This ensures each relation gets its own
// copy with independent referencedColumn and type properties.
relationalColumn = OrmUtils.cloneObject(relationalColumn)
}
relationalColumn.referencedColumn = referencedColumn // its important to set it here because we need to set referenced column for user defined join column
relationalColumn.type = referencedColumn.type // also since types of relational column and join column must be equal we override user defined column type

View File

@ -92,6 +92,20 @@ export class OrmUtils {
return target
}
/**
* Creates a shallow copy of the object, without invoking the constructor
*/
public static cloneObject<T extends object>(object: T): T {
if (object === null || object === undefined) {
return object
}
return Object.assign(
Object.create(Object.getPrototypeOf(object)),
object,
)
}
/**
* Deep compare objects.
*

View File

@ -0,0 +1,13 @@
import { Column, Entity, PrimaryColumn } from "../../../../../src"
@Entity()
export class City {
@PrimaryColumn()
name: string
@PrimaryColumn()
countryName: string
@Column()
population: number
}

View File

@ -0,0 +1,32 @@
import {
Column,
Entity,
JoinColumn,
ManyToOne,
PrimaryColumn,
} from "../../../../../src"
import { City } from "./City"
import { Country } from "./Country"
@Entity()
export class Company {
@PrimaryColumn()
name: string
@Column()
countryName: string
@ManyToOne(() => Country)
@JoinColumn({ name: "countryName" })
country?: Country
@Column()
cityName: string
@ManyToOne(() => City)
@JoinColumn([
{ name: "cityName", referencedColumnName: "name" },
{ name: "countryName", referencedColumnName: "countryName" },
])
city?: City
}

View File

@ -0,0 +1,10 @@
import { Column, Entity, PrimaryColumn } from "../../../../../src"
@Entity()
export class Country {
@PrimaryColumn()
name: string
@Column()
region: string
}

View File

@ -0,0 +1,98 @@
import { expect } from "chai"
import { DataSource } from "../../../../src"
import {
closeTestingConnections,
createTestingConnections,
reloadTestingDatabases,
} from "../../../utils/test-utils"
import { City } from "./entity/City"
import { Country } from "./entity/Country"
import { Company } from "./entity/Company"
describe("metadata builder > RelationJoinColumnBuilder", () => {
let dataSources: DataSource[]
before(
async () =>
(dataSources = await createTestingConnections({
entities: [__dirname + "/entity/*{.js,.ts}"],
})),
)
beforeEach(() => reloadTestingDatabases(dataSources))
after(() => closeTestingConnections(dataSources))
it("should not throw error when loading entities with composite FK with shared columns", () =>
Promise.all(
dataSources.map(async (dataSource) => {
await dataSource.getRepository(Country).save([
{ name: "Texas", region: "USA" },
{ name: "France", region: "EU" },
] satisfies Country[])
await dataSource.getRepository(City).save([
{
name: "Paris",
countryName: "France",
population: 2_100_000,
},
{
name: "Paris",
countryName: "Texas",
population: 25_000,
},
{
name: "Strasbourg",
countryName: "France",
population: 270_000,
},
{
name: "Lyon",
countryName: "France",
population: 720_000,
},
{
name: "Houston",
countryName: "Texas",
population: 2_300_000,
},
] satisfies City[])
await dataSource.getRepository(Company).save([
{ name: "NASA", countryName: "Texas", cityName: "Houston" },
{ name: "AXA", countryName: "France", cityName: "Paris" },
] satisfies Company[])
const companies = await dataSource.getRepository(Company).find({
relations: { city: true, country: true },
order: { name: "asc" },
})
expect(companies).to.deep.members([
{
name: "AXA",
countryName: "France",
cityName: "Paris",
city: {
countryName: "France",
name: "Paris",
population: 2_100_000,
},
country: { name: "France", region: "EU" },
},
{
name: "NASA",
countryName: "Texas",
cityName: "Houston",
city: {
countryName: "Texas",
name: "Houston",
population: 2_300_000,
},
country: { name: "Texas", region: "USA" },
},
] satisfies Company[])
}),
))
})

View File

@ -160,4 +160,42 @@ describe(`OrmUtils`, () => {
expect(result.foo).to.equal(foo)
})
})
describe("cloneObject", () => {
it("should create a shallow copy of an instance without invoking the constructor", () => {
class SomeClass {
static hasConstructorBeenInvoked = false
constructor(
public someString: string,
public someNumber: number,
) {
if (SomeClass.hasConstructorBeenInvoked) {
throw Error(
"The constructor was invoked a second time!",
)
}
SomeClass.hasConstructorBeenInvoked = true
}
clone() {
return new SomeClass(this.someString, this.someNumber)
}
}
const obj = new SomeClass("string", 0)
let objCopy: SomeClass | undefined
let objCopy2: SomeClass | undefined
expect(() => {
objCopy = OrmUtils.cloneObject(obj)
}).not.to.throw()
expect(() => {
objCopy2 = obj.clone()
}).to.throw()
expect(objCopy).not.to.equal(obj)
expect(objCopy).to.deep.equal(obj)
expect(objCopy2).to.equal(undefined)
})
})
})