-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
SQL Server Migrations: Idempotent scripts fails with 'invalid column name' (needs EXEC) #12911
Comments
Note for triage: I was able to reproduce this both in SQL Management Studio and when running the commands directly from ADO.NET:
ADO.NET repro code: public class BloggingContext : DbContext
{
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder
.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Test;ConnectRetryCount=0");
}
public class Program
{
private static readonly string[] migrations =
{
@"IF OBJECT_ID(N'[__EFMigrationsHistory]') IS NULL
BEGIN
CREATE TABLE [__EFMigrationsHistory] (
[MigrationId] nvarchar(150) NOT NULL,
[ProductVersion] nvarchar(32) NOT NULL,
CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId])
);
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192355_Initial')
BEGIN
CREATE TABLE [Blog] (
[Id] int NOT NULL IDENTITY,
CONSTRAINT [PK_Blog] PRIMARY KEY ([Id])
);
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192355_Initial')
BEGIN
CREATE TABLE [Post] (
[Id] int NOT NULL IDENTITY,
[BlogId] int NULL,
CONSTRAINT [PK_Post] PRIMARY KEY ([Id]),
CONSTRAINT [FK_Post_Blog_BlogId] FOREIGN KEY ([BlogId]) REFERENCES [Blog] ([Id]) ON DELETE NO ACTION
);
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192355_Initial')
BEGIN
CREATE INDEX [IX_Post_BlogId] ON [Post] ([BlogId]);
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192355_Initial')
BEGIN
INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion])
VALUES (N'20180810192355_Initial', N'2.1.1-rtm-30846');
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192452_AddName')
BEGIN
ALTER TABLE [Blog] ADD [Name] nvarchar(max) NULL;
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192452_AddName')
BEGIN
INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion])
VALUES (N'20180810192452_AddName', N'2.1.1-rtm-30846');
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192552_AddIndex')
BEGIN
DECLARE @var0 sysname;
SELECT @var0 = [d].[name]
FROM [sys].[default_constraints] [d]
INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id]
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[Blog]') AND [c].[name] = N'Name');
IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [Blog] DROP CONSTRAINT [' + @var0 + '];');
ALTER TABLE [Blog] ALTER COLUMN [Name] nvarchar(450) NULL;
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192552_AddIndex')
BEGIN
CREATE UNIQUE INDEX [IX_Blog_Name] ON [Blog] ([Name]) WHERE [Name] IS NOT NULL;
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192552_AddIndex')
BEGIN
INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion])
VALUES (N'20180810192552_AddIndex', N'2.1.1-rtm-30846');
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192708_RemoveName')
BEGIN
DROP INDEX [IX_Blog_Name] ON [Blog];
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192708_RemoveName')
BEGIN
DECLARE @var1 sysname;
SELECT @var1 = [d].[name]
FROM [sys].[default_constraints] [d]
INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id]
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[Blog]') AND [c].[name] = N'Name');
IF @var1 IS NOT NULL EXEC(N'ALTER TABLE [Blog] DROP CONSTRAINT [' + @var1 + '];');
ALTER TABLE [Blog] DROP COLUMN [Name];
END;
",
@"IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20180810192708_RemoveName')
BEGIN
INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion])
VALUES (N'20180810192708_RemoveName', N'2.1.1-rtm-30846');
END;
"
};
public static void Main()
{
using (var context = new BloggingContext())
{
context.Database.EnsureDeleted();
context.Database.EnsureCreated();
var connection = context.Database.GetDbConnection();
Console.WriteLine("Migrating once:");
ApplyMigrations(connection);
Console.WriteLine("Migrating twice:");
ApplyMigrations(connection);
}
}
private static void ApplyMigrations(DbConnection connection)
{
connection.Open();
foreach (var migration in migrations)
{
var command = connection.CreateCommand();
command.CommandText = migration;
command.ExecuteNonQuery();
}
connection.Close();
}
} |
Triage: for 3.0, we will investigate the minimal set of places where we have to wrap in an exec call to avoid the SQL Server parsing issue. Worst case, we will have to do it everywhere. Note that this should only be done when generating idempotent scripts. |
@christianholsen can you paste a gist with a powershell script that you use to prepare migration file? |
Yep. As a wrote above the only problem I found was the specific scenario specific where I have created an index for a nullable column and later remove it. The problem arises when the CREATE UNIQUE INDEX statement is evaluated even though it is not executed. So I have chosen to wrap all CREATE UNIQUE INDEX statements in the SQL in EXEC blocks. This has the effect that the script is dynamic and will only be evaluated when it actually is executed - fixing the immediate problem. I use Team City as my build tools. You should be able to use this approach with any build tool. I have a build step where I create the idempotent SQL file as mentioned in my first post. This step is executed as a command line step using the dotnet cli : dotnet ef migration script...etc (see my first post) This creates the idempotent script 'migrationscript.sql' Next I have a powershell script step where I replace CREATE UNIQUE INDEX using regular expressions.
As I understand it this problem will be fixed at some point in EF CORE, so this is working fine for me for now. :-) Hope this helps, |
@christianholsen Thanks for response. I found different solution. I am producing all migrations separately and in Octopus Deploy run only those migrations, that were not applied. So I am waiting as well when this will be fixed, so I can switch back to migrations.sql. :) |
We are experiencing the same issue with hand written updates after column was removed/renamed: |
We are also experiencing this issue. The workaround that @christianholsen used seems to have gotten us past this, as we are using Azure DevOps for deployments we were able to patch the script before we release. |
The same is occurring to us with Idempotent Script Generation. In our case it is not happening with an INDEX creation, but with an IDENTITY INSERT. |
Another project where this problem occurs (without involving indexes) is the current version of the Contoso University sample app (available here https://github.com/aspnet/Docs/tree/0ee6b101d9d4b4022add3583ed25f0d89674b87b/aspnetcore/data/ef-mvc/intro/samples/cu-final). The Unfortunately in this case, wrapping the instructions in an IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20170816195930_Inheritance')
BEGIN
UPDATE dbo.Enrollment SET StudentId = (SELECT ID FROM dbo.Person WHERE OldId = Enrollment.StudentId AND Discriminator = 'Student')
END; |
I'm having the same problem with the migrations. When the idempotent flag is set not all statements are wrapped with a It there an ETA know for this issue to be resolved? |
We still hope to address this issue before the final 3.0.0 release (this September). |
We are having this issue too and it would be nice to have it fixed for 2.x too |
For those using Azure DevOps, we did release a set of .NET Core Pipeline tasks. A temporary workaround for this issue is included as an option in our tasks that script the migrations (https://marketplace.visualstudio.com/items?itemName=iowacomputergurus.dotnetcore-pipeline-tasks) Just a workaround until this can be truly fixed |
Hi, Another workaround is to globally wrap SQL scripts with EXECUTE() statement. In case of someone looking for such an approach here is very dumb (roughly tested) implementation:
Then somewhere in your IDesignTimeDbContextFactory implementation: |
I came here looking for issues around creating VIEWs and TRIGGERs with an According to the SQL laws laid down by Microsoft, IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20190617133214_Add_Crossing_Coordinate_View')
BEGIN
CREATE VIEW [dbo].[crossing_coordinate_view]
AS
SELECT Crossing.Id, This incurs More info here: This ultimately makes dotnet ef migrations script unusable for release automation, arguably it's principal purpose. Please fix (one solution is to emit a |
Ok, I've worked around this by using a powershell task in my release pipeline to munge the migration script in the artifact drop folder before executing it. I'm using regex to wrap all $sql = get-content .\migrate.sql -raw
[regex]::replace($sql, "BEGIN\s+(CREATE (?:VIEW|TRIGGER).+?)END", "BEGIN`nEXEC('`$1');`nEND", "ignorecase,singleline") > migrate.sql Make sure you tick the box to use PowerShell Core (for the |
For those who uses idempotent script to apply the migrations. This approach splits the script on separate queries by "GO" statements an checks if this query is a part of the migration that was already applied.
NOTE: When you make this query, make sure to check if migration table exists first.
Good things about this solution:
Note: it will not filter the very first SQL query:
But this query is completely safe and will not cause any problems like discussed in this issue. |
Also including |
We also wrap all of our |
@ChristopherHaws Good strategy. We should mention this in the docs. Filed dotnet/EntityFramework.Docs#2561 |
@bricelam Any plans to backport fix to 3.1.x branch? |
No, sorry. This fix required too significant of changes for a patch release—especially on an LTS release. |
I've found another workaround to this issue by using a powershell script to edit the Idempotent script prior to executing it. It fetches the list of MigrationIds and comments out the relevant script blocks.
|
It'd also be nice to have Thanks for the fix already made! It'll avoid the error in case of |
I am having the same issue. Was there any fix or work around for this? Though Idempotent script, sql seems to evaluate the query, though it will not get executed. And I am getting an error "Invalid column name". Any idea? |
@bricelam what about net5.0? Still doesn't work. |
Upgraded to 5.0 and it worked.
Kind Regards,
Vid
…________________________________
From: Slava Trenogin ***@***.***>
Sent: Sunday, March 14, 2021 7:43:25 AM
To: dotnet/efcore ***@***.***>
Cc: Vid Sankar ***@***.***>; Comment ***@***.***>
Subject: Re: [dotnet/efcore] SQL Server Migrations: Idempotent scripts fails with 'invalid column name' (needs EXEC) (#12911)
@bricelam<https://github.com/bricelam> what about net5.0? Still doesn't work.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub<#12911 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AG2FGENBB3JCS4ABVVU3OFDTDRSJ3ANCNFSM4FOGIKTA>.
|
@derigel23 Can you create a new issue with more details about the issue you're encountering? |
@bricelam I've found out that errors are rising from my custom SQL statements in migration builder. |
Escape them manually. Users are responsible for ensuring their custom SQL statements work with the idempotent script. (Since we generally avoid trying to parse SQL in EF) |
Super useful, great idea, we're building the idempotent script and publishing as an artifact. At the time of build we can't determine which migrations have been applied, just when we're running the release. Should resolve the problem described in this issue about the invalid column name, which we're experiencing 👍. Worth noting that this only happens when we're running the release through Azure DevOps pipelines. It works fine If I take the script and run it through MSSMS. |
We have been using this extension method in place of /// <summary>
/// Executes the SQL statement via sp_executesql which does not get validated until runtime.
/// </summary>
public static OperationBuilder<SqlOperation> ExecuteSql(this MigrationBuilder migrationBuilder, string sql) =>
migrationBuilder.Sql($"EXEC sp_executesql N'{sql.Replace("'", "''")}'"); |
@ChristopherHaws thanks for that, it solved our problem 👍 We had issues at first, because the generated SQL script didn't properly close the ' (single quotes) correctly. After some digging, we found out that our raw SQL scripts sometimes contained Example
Apparently EF Core splits the scripts based on The workaround for us was to extend your method a bit: /// <summary>
/// Executes the SQL statement via sp_executesql which does not get validated until runtime. This avoids problems
/// when generating an idempotent SQL script with the command `dotnet ef migrations script` (see
/// https://github.com/dotnet/efcore/issues/12911#issuecomment-1142441149)
///
/// NOTE: When this method is called on an SQL statement that contains `GO`, EF Core splits the statements at those
/// and creates separate steps in the migrations. Unfortunately this will lead to a broken SQL script, as there are
/// no closing ' (single quote) characters. So we comment out the GO statements dynamically.
/// </summary>
public static OperationBuilder<SqlOperation> ExecuteSqlSafely(this MigrationBuilder migrationBuilder, string sql)
{
const string pattern = "^\\s*GO;?\\s*$";
const string replacement = $"-- GO (Commented out by {nameof(ExecuteSqlSafely)} method)";
// First comment out all lines that contain only a `GO` statement (case insensitive, with optional semicolon and
// whitespace). This requires the regex to be run in Multiline mode
var modifiedSql = Regex.Replace(sql, pattern, replacement, RegexOptions.Multiline | RegexOptions.IgnoreCase);
// Then escape all single quotes ('), which is necessary because of EXEC sp_executesql N''
modifiedSql = modifiedSql.Replace("'", "''");
return migrationBuilder.Sql($"EXEC sp_executesql N'{modifiedSql}'");
} |
Isn't it possible to use |
@bricelam,
|
@a-witkowski Can you file a new issue? |
I wonder if this could just be added to the framework. We are adding this to our projects and going to start exclusively using it too because of dropped columns in a similar situation. Getting frustrated it keeps randomly popping up in different circumstances. Is there a security risk or something to not have the |
The payload of that is specific to SQL Server. I don't think there's particularly high overhead to putting that into a project yourself, personally. It's less of a security concern and more of a behavioural quirk, I'd say mainly because this is largely dependent on the engine you're targeting. |
I have encountered a problem with idempotent migration scripts that make them fail in our continous integration system. The reason is that some migration scripts are parsed even though they are not going to be executed.
This issue is much like issue #10717. This issue can be reproduced by manipulating EF types i a sequence of migrations.
In example I have these two migrations, the first is adding a unique index for a nullable column, the second is removing the column:
This will work fine in the first execution, just as #10717
The second time this in run in our CI system, MyTable will no longer have the column "Name" and will fail in the execution of
The SQL error message is
This happens even though the migration "AddIndexToMyTable" has been installed and the "If not exists.." statement should avoid execution of the script, but as i happens it is parsed anyways, making it fail!
Steps to reproduce
Proposal for a solution
This problem only occurs because the script section is parsed in the sql server even though it is not going to be executed. If this could be avoided the problem would not occur. It could be solved by using dynamic a sql script, as:
As it is i'm adding the exec to the migration script with powershell in CI, but I believe that everyone would be happier if we could rely on the script produced by EFCore :-)
Further technical details
EF Core version: 2.1.1
The text was updated successfully, but these errors were encountered: