dotnet / ef6

This is the codebase for Entity Framework 6 (previously maintained at https://entityframework.codeplex.com). Entity Framework Core is maintained at https://github.com/dotnet/efcore.
https://docs.microsoft.com/ef/ef6
MIT License
1.43k stars 545 forks source link

Migration script issue #257

Closed PhilPJL closed 7 years ago

PhilPJL commented 7 years ago

If I have a migration like this

public partial class EnableAutoUnload : DbMigration
{
    public override void Up()
    {
        AddColumn("dbo.ProductionLine", "EnableAutoUnload", c => c.Boolean(nullable: false));
        AddColumn("dbo.ProductionLine", "AutoUnloadDoffNumber", c => c.Int());

        Sql("update dbo.ProductionLine set EnableAutoUnload = 1 where MaxSpoolsOnWinder > 1");
    }

If I run this from the package manager console it works.

ALTER TABLE [dbo].[ProductionLine] ADD [EnableAutoUnload] [bit] NOT NULL DEFAULT 0 ALTER TABLE [dbo].[ProductionLine] ADD [AutoUnloadDoffNumber] [int] update dbo.ProductionLine set EnableAutoUnload = 1 where MaxSpoolsOnWinder > 1 INSERT [dbo].[__MigrationHistory]([MigrationId], [ContextKey], [Model], [ProductVersion]) VALUES (...

If I generate the script (above) and run that it doesn't, because it needs a 'go' before the update statement.

If I try adding Sql('go') or Sql('go update ...'

etc then I get The argument 'sql' cannot be null, empty or contain only white space when trying to create the script in the package manager console.

The only thing to do is generate the script and modify it afterwards.

Bug?

dschuermans commented 7 years ago

@PhilPJL - Try wrapping your Sql() statements in EXECUTE('') statements...

So Sql ("Update table SET Column = 'Value'") becomes Sql("EXECUTE('Update table SET Column = ''Value''')")

Keep in mind the escaped single quotes around value

CZEMacLeod commented 7 years ago

These extension methods may help...

Imports System.Runtime.CompilerServices
Imports System.Data.Entity.Migrations.Model

Namespace Global.System.Data.Entity.Migrations

    Public Module SqlExecuteDbMigrationExtensions
        <Extension>
        Public Function SqlExecute(sql As String) As String
            Return "EXECUTE ('" & sql.Replace("'", "''") & "');"
        End Function

        <Extension>
        Public Sub SqlExecute(migration As DbMigration, sql As String)
            DirectCast(migration, Infrastructure.IDbMigration).AddOperation(New Model.SqlOperation(sql.SqlExecute()))
        End Sub
    End Module
End Namespace

So Sql("Update table SET Column = 'Value'") becomes SqlExecute("Update table SET Column = 'Value'")

PhilPJL commented 7 years ago

Yes of course, thanks. Should have thought of that :)

adrianhr91 commented 7 years ago

Shouldn't this be marked as a bug? The PowerShell tools for applying migrations are behaving differently to the SQL script (that they generate as an alternative). Which you wouldn't realise until it's too late.

CZEMacLeod commented 7 years ago

@adrianhr91 The PowerShell tools and the built in migrations commands execute each step separately on the same connection within a transaction (if memory serves). This is why they don't need to worry about GO statements etc. You can also run these migrations using a command line tool (but I can't remember what it is called just now) which would be a good way of applying updates using higher privileges than the normal application or website would have. The SQL Scripts generated are a mechanism that you can use to output files to hand to a DBA and are not the 'normal' operation - I believe it even states that these should only be used as a basis for an upgrade script. I believe there is also a (slight) difference between running this kind of script in SqlCmd mode vs. in e.g. SSMS. Also note that the issue here comes mainly from mixing DDL and DML commands. I think that the SqlExecute extension method should probably be part of the core so it could be used by default for DML commands (which are manually added anyway). I quite often use it myself when transferring data from one column or table to another midway through a migration. E.g. after the new column has been added, but before the old one is deleted. E.g. replacing a bool/bit flag with an enum value. I'm not sure what you mean by 'until it's too late' as I would always be doing a test run of any scripts and the generated migrations in general before shipping. I'm always happy to hear a different perspective though.

adrianhr91 commented 7 years ago

@CZEMacLeod My current development/release process is based on the assumption that running migrations through PowerShell and through a generated SQL script would produce the same result. So, for convenience, when developing locally I would usually use PowerShell to execute migrations and then generate a SQL script that is easier to run in an automated process. So by 'too late' I meant I wouldn't notice an issue with a migration (similar to the one described in the first comment) until it fails to deploy on an environment other than local.

Is there a particular reason to want the PowerShell tools to behave differently to the generated SQL script in particular that I am missing? Or is it more of a technical issue to align them?

CZEMacLeod commented 7 years ago

@adrianhr91 Ah, I see. - PowerShell wraps the same calls that migrate.exe does, which is basically the same as the Automatically Upgrading on Application Startup (MigrateDatabaseToLatestVersion Initializer) process. The bit that talks about SQL Scripts does say that they are for handing off to a DBA. I am not sure what your automated process is, but I wonder if you could use migrate.exe or just the built in migrate on startup instead - it seems to be the default way of doing things with migrations and is what we use. I normally only use the SQL scripts if I need to debug the upgrade or find an issue with the migration applying (often because of bad data).

I'll let someone with more knowledge than I address how to make sure the SQL Script works the same as the built in mechanism.