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

Idempotent migration script not working on EFCore7.0 for AlterColumn when nullable changed from true to false #29530

Closed
PTAdvanced opened this issue Nov 10, 2022 · 1 comment · Fixed by #29619
Assignees
Labels
area-migrations area-sqlserver closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@PTAdvanced
Copy link

After upgrading to .NET7 a Azure DevOps pipeline failed. Investigating I found that the idempotent migration script we generate to be applied to the target Azure SQL Database is failing to be applied, with the error

##[error]Invalid column name 'CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds'.

Include your code

Generating the idempotent migration script locally using .NET6 and then .NET7 reveals a difference in the generated SQL migration code.

I have a migration definition defined as

        migrationBuilder.AlterColumn<double>(
            name: "CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds",
            table: "SitesSet",
            type: "float",
            nullable: false,
            defaultValue: 0.0,
            oldClrType: typeof(double),
            oldType: "float",
            oldNullable: true);

Essentially the property is no longer nullable, and has a default value defined.

in .NET6 this migration was generated as

IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20220127150855_AddedOccurredAtToFireSystemStateAndInvestigationDelayState')
BEGIN
    DECLARE @var305 sysname;
    SELECT @var305 = [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'[SitesSet]') AND [c].[name] = N'CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds');
    IF @var305 IS NOT NULL EXEC(N'ALTER TABLE [SitesSet] DROP CONSTRAINT [' + @var305 + '];');
    ALTER TABLE [SitesSet] ALTER COLUMN [CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds] float NOT NULL;
    ALTER TABLE [SitesSet] ADD DEFAULT 0.0E0 FOR [CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds];
END;
GO

in .NET7 this is generated as

IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20220127150855_AddedOccurredAtToFireSystemStateAndInvestigationDelayState')
BEGIN
    DECLARE @var305 sysname;
    SELECT @var305 = [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'[SitesSet]') AND [c].[name] = N'CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds');
    IF @var305 IS NOT NULL EXEC(N'ALTER TABLE [SitesSet] DROP CONSTRAINT [' + @var305 + '];');
    UPDATE [SitesSet] SET [CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds] = 0.0E0 WHERE [CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds] IS NULL;
    ALTER TABLE [SitesSet] ALTER COLUMN [CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds] float NOT NULL;
    ALTER TABLE [SitesSet] ADD DEFAULT 0.0E0 FOR [CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds];
END;
GO

The difference is in the .NET7 migration there is a

UPDATE [SitesSet] SET [CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds] = 0.0E0 WHERE [CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds] IS NULL;

I understand why this has been added, as previously I have created SQL updates like this to ensure any existing rows have the default applied where the current value is NULL.

However, in this situation the column name has since been changed in a later migration. Therefore when running the SQL script we encounter the error

Invalid column name 'CurrentSiteInvestigationDelayStatus_InvestigationDelayInSeconds'.

The UPDATE statement should be wrapped in an EXEC(''); as recommended in the official docs here Arbitrary changes via raw SQL

"EXEC is used when a statement must be the first or only one in a SQL batch. It can also be used to work around parser errors in idempotent migration scripts that can occur when referenced columns don't currently exist on a table."

Include provider and version information

EF Core version:
Database provider: (e.g. Microsoft.EntityFrameworkCore.SqlServer)
Target framework: (e.g. .NET 7.0)
Operating system: WIndows 11
IDE: (e.g. Visual Studio 2022 17.4)

@ajcvickers
Copy link
Contributor

/cc @bricelam

roji added a commit to roji/efcore that referenced this issue Nov 19, 2022
@roji roji self-assigned this Nov 19, 2022
@roji roji added this to the 8.0.0 milestone Nov 19, 2022
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Nov 19, 2022
@roji roji removed this from the 8.0.0 milestone Nov 19, 2022
@ajcvickers ajcvickers added this to the 7.0.x milestone Dec 6, 2022
@ajcvickers ajcvickers reopened this Dec 6, 2022
roji added a commit to roji/efcore that referenced this issue Dec 6, 2022
roji added a commit to roji/efcore that referenced this issue Dec 7, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.x, 7.0.3 Dec 8, 2022
roji added a commit to roji/efcore that referenced this issue Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations area-sqlserver closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants