Skip to content

Primary keys#21

Merged
PhenX merged 14 commits intoPhenX:masterfrom
SebastianStehle:primary-keys
May 22, 2025
Merged

Primary keys#21
PhenX merged 14 commits intoPhenX:masterfrom
SebastianStehle:primary-keys

Conversation

@SebastianStehle
Copy link
Copy Markdown
Collaborator

No description provided.

@SebastianStehle SebastianStehle requested a review from PhenX as a code owner May 22, 2025 09:53
@SebastianStehle
Copy link
Copy Markdown
Collaborator Author

SebastianStehle commented May 22, 2025

This is a follow up after #19

I recommend not to have a look into it, until the other one is merged.

What I have done:

  • Move stuff to the dialect (CreateTableCopySql) and create a method for that.
  • Added another test.
  • Move some stuff to the connection-info class to make the provider base easier.
  • Improve performance of SQL building by using string builder properly and getting rid of string.Join to save allocations.
  • Automatically use the primary key (when defined) for conflict detection, e.g. you can just do
await _context.ExecuteBulkInsertAsync(entities,
    onConflict: new OnConflictOptions<TestEntityWithGuidId>
    {
        Update = e => e,
    });

It only works with non-generated PKs, otherwise SQL server causes issues.

@SebastianStehle
Copy link
Copy Markdown
Collaborator Author

@PhenX Ready to be reviewed

Comment thread src/PhenX.EntityFrameworkCore.BulkInsert.SqlServer/SqlServerDialectBuilder.cs Outdated

public string QuotedColumName { get; } = dialect.Quote(property.GetColumnName());

public string StoreDefinition { get; } = GetStoreDefinition(property);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is it used ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not anymore. I tested some stuff with SQL Server and whether it makes more sense to create the temp table manually.

The point is that the identity constraints are somehow copied to the SQL table, therefore you cannot make upserts when you have a auto-incremented ID. I still want to continue experimenting with that.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used now. I create the temp table manually for SQL server now, therefore you can make upserts with auto-incremented IDs now.

@SebastianStehle
Copy link
Copy Markdown
Collaborator Author

From my side it is ready to be merged.

@PhenX PhenX merged commit 5f2a3a6 into PhenX:master May 22, 2025
1 check passed
@SebastianStehle SebastianStehle deleted the primary-keys branch May 22, 2025 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants