Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simple insert fails when key col has an explicit collation #28317

Closed
clement911 opened this issue Jun 26, 2022 · 7 comments
Closed

Simple insert fails when key col has an explicit collation #28317

clement911 opened this issue Jun 26, 2022 · 7 comments

Comments

@clement911
Copy link
Contributor

clement911 commented Jun 26, 2022

Minimum repro: https://github.com/clement911/EfCollationBugRepro

EF Core version :6.0.6
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 6.0

Scenario:

  1. Create an entity with a leading key that has a collation different from the default DB collation.
  2. Attempt to insert 4 entities in one batch
  3. Observe Collation error being throw

Please see attached repo.

When EF use batching for inserts (e.g. inserting more than 4 entities, by default), EF issues a MERGE statement which results in a collation conflict error.

Here is the sql generated by EF. I highlighted the part that causes the error.

image

The error occurs because of a collation conflict.
The physical column MyEntity.KeyCol1 has collation Latin1_General_100_BIN2
However, the table variable KeyCol1 column has no explicit collation so it defaults to the DB's default collation (SQL_Latin1_General_CP1_CI_AS in our case).
As a result, the equality predicate fails.

Fix:
The generated sql should assign the correct collation to the table variable columns.

See below. We have verified manually that this fixes the issues (i.e. the statement runs succesfully)

image

@roji
Copy link
Member

roji commented Jun 27, 2022

The above specifically should no longer happen in EF 7, since we no longer use a temporary table variable and select back from it (instead we just do MERGE with OUTPUT directly). You can use the latest preview to check.

But the above is still relevant for when the table has triggers - the user must then tell EF to use the older, less efficient technique above, where the collation issue would exist.

As a workaround, you can disable the MERGE statement by setting your max batch size to 1 (though this will degrade performance by adding more roundtrips).

@clement911
Copy link
Contributor Author

That's great for EF7.
Will there be no fix for EF6?

@clement911
Copy link
Contributor Author

Besides the suggested work around, but we'd rather keep the batching functionality to get optimal perf.

@roji
Copy link
Member

roji commented Jun 28, 2022

Will there be no fix for EF6?

It's unlikely that this would meet the bar for a patch, but we'll discuss this in triage.

@AndriySvyryd AndriySvyryd added closed-no-further-action The issue is closed and no further action is planned. closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed closed-no-further-action The issue is closed and no further action is planned. labels Jun 28, 2022
@AndriySvyryd AndriySvyryd added this to the 7.0.0 milestone Jun 28, 2022
@AndriySvyryd
Copy link
Member

We discussed this in triage and fixing this in 6 would be too risky.

@ajcvickers ajcvickers reopened this Jun 29, 2022
@ajcvickers ajcvickers removed this from the 7.0.0 milestone Jun 29, 2022
@AndriySvyryd
Copy link
Member

Covered by #7172

@AndriySvyryd AndriySvyryd closed this as not planned Won't fix, can't repro, duplicate, stale Jun 29, 2022
@AndriySvyryd AndriySvyryd added closed-duplicate and removed closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. labels Jun 29, 2022
@roji roji added this to the 7.0.0 milestone Jun 29, 2022
@roji
Copy link
Member

roji commented Jun 29, 2022

Thanks @AndriySvyryd, did not realize we had another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants