Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 55 additions & 3 deletions src/query-builder/InsertQueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,10 @@
!DriverUtils.isMySQLFamily(this.dataSource.driver) &&
!(this.dataSource.driver.options.type === "aurora-mysql") &&
!(
this.dataSource.driver.options.type === "mssql" &&
(this.dataSource.driver.options.type === "mssql" ||
DriverUtils.isPostgresFamily(
this.dataSource.driver,
)) &&
this.isOverridingAutoIncrementBehavior(column)
)
)
Expand Down Expand Up @@ -1239,6 +1242,19 @@
if (this.dataSource.driver.options.type === "mssql") {
query += `;`
}

// Inserting a specific value for an auto-increment primary key in mssql requires enabling IDENTITY_INSERT
// IDENTITY_INSERT can only be enabled for tables where there is an IDENTITY column and only if there is a value to be inserted (i.e. supplying DEFAULT is prohibited if IDENTITY_INSERT is enabled)
if (
this.connection.driver.options.type === "mssql" &&

Check warning on line 1249 in src/query-builder/InsertQueryBuilder.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'connection' is deprecated.

See more on https://sonarcloud.io/project/issues?id=typeorm_typeorm&issues=AZ17z6-91Rn55vnUUI3g&open=AZ17z6-91Rn55vnUUI3g&pullRequest=11820
this.expressionMap.mainAlias!.hasMetadata &&

Check warning on line 1250 in src/query-builder/InsertQueryBuilder.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=typeorm_typeorm&issues=AZ17z6-91Rn55vnUUI3h&open=AZ17z6-91Rn55vnUUI3h&pullRequest=11820
columns.some((column) =>
this.isOverridingAutoIncrementBehavior(column),
)
) {
query = `SET IDENTITY_INSERT ${tableName} ON; ${query}; SET IDENTITY_INSERT ${tableName} OFF`
}

return query
}

Expand All @@ -1247,10 +1263,44 @@
*
* @param mergeSourceAlias
*/
protected createMergeIntoSourceExpression(

Check failure on line 1266 in src/query-builder/InsertQueryBuilder.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=typeorm_typeorm&issues=AZ17z6-91Rn55vnUUI3i&open=AZ17z6-91Rn55vnUUI3i&pullRequest=11820
mergeSourceAlias: string,
): string {
const columns = this.getInsertedColumns()
let columns = this.getInsertedColumns()

// For MERGE INTO, we need to include conflict path columns in the source
// even if they are auto-generated (e.g., identity columns in SAP HANA)
// because they are needed for the ON clause (conflict detection)
if (
this.expressionMap.onUpdate?.conflict &&
this.expressionMap.mainAlias!.hasMetadata

Check warning on line 1276 in src/query-builder/InsertQueryBuilder.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=typeorm_typeorm&issues=AZ17z6-91Rn55vnUUI3j&open=AZ17z6-91Rn55vnUUI3j&pullRequest=11820
) {
const conflictPaths = Array.isArray(
this.expressionMap.onUpdate.conflict,
)
? this.expressionMap.onUpdate.conflict
: [this.expressionMap.onUpdate.conflict]

const conflictColumns =
this.expressionMap.mainAlias!.metadata.columns.filter(

Check warning on line 1285 in src/query-builder/InsertQueryBuilder.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=typeorm_typeorm&issues=AZ17z6-91Rn55vnUUI3k&open=AZ17z6-91Rn55vnUUI3k&pullRequest=11820
(column) =>
conflictPaths.includes(column.databaseName) ||
conflictPaths.includes(column.propertyPath),
)

// Add conflict columns that are not already in the inserted columns
const additionalColumns = conflictColumns.filter(
(conflictColumn) =>
!columns.some(
(col) =>
col.databaseName === conflictColumn.databaseName,
),
)

if (additionalColumns.length > 0) {
columns = [...columns, ...additionalColumns]
}
}

let expression = "USING ("

Expand Down Expand Up @@ -1430,7 +1480,9 @@
(column.isGenerated &&
column.generationStrategy === "uuid" &&
this.dataSource.driver.isUUIDGenerationSupported()) ||
(column.isGenerated && column.generationStrategy !== "uuid")
(column.isGenerated &&
column.generationStrategy !== "uuid" &&
!this.isOverridingAutoIncrementBehavior(column))
) {
expression += `DEFAULT`
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,35 @@ describe("repository > basic methods", () => {
.should.be.rejectedWith(TypeORMError)
}),
))
it("should upsert with primary key as conflict path", () =>
Promise.all(
connections.map(async (connection) => {
if (!connection.driver.supportedUpsertTypes.length) return

const postRepository = connection.getRepository(Post)

// First upsert - should insert a new post with id=1
await postRepository.upsert(
{ id: 1, title: "Upserted post" },
{ conflictPaths: ["id"] },
)

// Second upsert - should update the existing post with id=1
await postRepository.upsert(
{ id: 1, title: "Upserted post2" },
{ conflictPaths: ["id"] },
)

// Verify only one post exists with the updated title
const posts = await postRepository.find()
posts.length.should.be.equal(
1,
"Should have only one post, not duplicate",
)
posts[0].id!.should.be.equal(1)
posts[0].title.should.be.equal("Upserted post2")
}),
))
})

describe("preload also should also implement merge functionality", function () {
Expand Down
Loading