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

[5.0.2] Migrations: Generate valid SQL when escaping new lines in seed data #23634

Merged
merged 1 commit into from
Dec 11, 2020

Conversation

smitpatel
Copy link
Contributor

@smitpatel smitpatel commented Dec 9, 2020

Resolves #23518
Resolves #23459

Description

We fixed a bug in EF Core 5.0 to preserve the formatting of seed data with embedded new lines: #20662. The fix involves using the SQL concat function, but this was done in a way that creates an unbalanced tree in the SQL which then fails in both SQL Server and SQLite for large amounts of data.

Customer Impact

Migrations of EnsureCreated with large, multi-line strings in seed data throw. The only workarounds are to remove the seed data and get it to the database some other way, or to strip out the newlines.

How found

Customer reported on 5.0.0 for both SQL Server and SQLite

Test coverage

We have added test coverage to ensure the tree is created in balanced way.

Regression?

Yes, from 3.1.

Risk

Low. Doesn't fundamentally change anything except the balancing of the tree. Quirked.

@smitpatel smitpatel requested a review from a team December 9, 2020 00:44
@smitpatel
Copy link
Contributor Author

Updated.

@ajcvickers ajcvickers added this to the 5.0.x milestone Dec 10, 2020
@ajcvickers ajcvickers changed the title Migrations: Generate valid SQL when escaping new lines in seed data [5.0.2] Migrations: Generate valid SQL when escaping new lines in seed data Dec 10, 2020
@ajcvickers
Copy link
Contributor

Approved by Tactics for 5.0.2.

@smitpatel smitpatel merged commit 0e91fa1 into release/5.0 Dec 11, 2020
@smitpatel smitpatel deleted the smit/treebalancing branch December 11, 2020 21:11
@ajcvickers ajcvickers removed this from the 5.0.2 milestone Dec 11, 2020
@saliksaly
Copy link

saliksaly commented Feb 10, 2021

Using CONCAT for strings is not so good due to string truncation to 4000 chars in SQL server :-(
Now the migrations run without error (Microsoft.Data.SqlClient.SqlException (0x80131904): The concat function requires 2 to 254 arguments.) but my seeded strings are now truncated in the database. Silently!

Here are some resources for this SQL server behavior:
https://stackoverflow.com/questions/4547821/using-varcharmax-with-string-concatenation-in-sql-server-2005
https://stackoverflow.com/questions/4833549/nvarcharmax-still-being-truncated
https://sqlundercover.com/2017/09/27/concatenation-truncation-are-your-strings-being-truncated/

Is there any workaround for this? We have had CSHTML templates in seed data and now it would not be posible in new version of .net?

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

Successfully merging this pull request may close these issues.

6 participants