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

EF core creates migration adding indentation white chars into value #15256

Closed
krzycho1024 opened this issue Apr 4, 2019 · 7 comments
Closed
Assignees
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-3.0 type-bug
Milestone

Comments

@krzycho1024
Copy link

EF core creates migration adding indentation white chars into value when generating .sql script.

image

Steps to reproduce

create migration:

builder.Property(x => x.Column).HasDefaultValue("Don't\r\n" +
                                                                 "GO\r\n" +
                                                                 "this way\r\n");

run: add-migration test
when creating .sql migration: dotnet ef migrations script -i
indentation is applied inside quoted value

Further technical details

EF Core version: 2.2.3
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10

@krzycho1024
Copy link
Author

image

@ajcvickers ajcvickers added this to the 3.0.0 milestone Apr 5, 2019
@bricelam
Copy link
Contributor

bricelam commented Apr 5, 2019

Tricky, tricky. This line would need more information about what can and can't be indented.

https://github.com/aspnet/EntityFrameworkCore/blob/610aa8608d3f912082c56a7d0e4f791cd9b6b211/src/EFCore.Relational/Migrations/Internal/Migrator.cs#L313-L319

The easiest fix would be to not indent the body of the IF statement... but that's ugly for everyone else.

@bricelam
Copy link
Contributor

bricelam commented Apr 5, 2019

The workaround is to use...

.HasDefaultValueSql(
    "N'Don''t' + CHAR(13) + CHAR(10) + N'GO' + CHAR(13) + CHAR(10) + N'this way'");

I suppose we could always generate string literals with newlines like this.

@bricelam
Copy link
Contributor

@roji The fix by @lauxjpn seems like the most reasonable way to fix this issue. Do you agree? It has to be done per provider, but anything else will probably require significant plumbing, might be breaking, and will also likely just require providers to update anyway.

@lajones
Copy link
Contributor

lajones commented Mar 26, 2020

Closed fixed by #20304

@lajones lajones closed this as completed Mar 26, 2020
@lajones lajones added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Mar 26, 2020
@lajones
Copy link
Contributor

lajones commented Mar 26, 2020

Note: I filed #20391 to track a similar issue for DefaultValueSql and ComputedColumnSql.

@roji
Copy link
Member

roji commented Aug 29, 2020

This is unfortunately a bit more complicated for PostgreSQL - there are contexts where only a true literal is valid. For example, this breaks test MigrationsNpgsqlTest.Create_table_with_multiline_comments, since the PG comment syntax doesn't accept expressions with concatenation operators.

It may be better to selectively apply the concatenation logic in NpgsqlMigrationsSqlGenerator where we know it's valid (e.g. default value), rather than broadly in the StringTypeMapping's literal generation logic.

@ajcvickers ajcvickers modified the milestones: 5.0.0-preview3, 5.0.0 Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-3.0 type-bug
Projects
None yet
Development

No branches or pull requests

7 participants