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

Existing Migration file which renames table from XXX to YYY and creates a new table XXX produces KeyNotFoundException #35323

Open
trancefreak77 opened this issue Dec 12, 2024 · 2 comments

Comments

@trancefreak77
Copy link

EF Core 9.0.0 seems to have introduced a bug which was not present in previous versions.
Our existing migration file has the following content:

using Microsoft.EntityFrameworkCore.Migrations;

namespace Authorization.Infrastructure.Migrations
{
    public partial class AdaptEntitiesForNewEndpoints : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropForeignKey(
                name: "FK_ModulePermission_Module_ModuleId",
                schema: "authorization",
                table: "ModulePermission");

            migrationBuilder.RenameTable(
                name: "Module",
                newName: "ModuleGroup",
                newSchema: "authorization",
                schema: "authorization");

            migrationBuilder.CreateTable(
              name: "Module",
              schema: "authorization",
              columns: table => new
                                {
                                  Id = table.Column<int>(type: "int", nullable: false)
                                    .Annotation("SqlServer:Identity", "1, 1"),
                                  ModuleId = table.Column<string>(type: "nvarchar(max)", nullable: true)
                                },
              constraints: table =>
              {
                table.PrimaryKey("PK_Module", x => x.Id)
                  .Annotation("SqlServer:Clustered", true);
              });

As you can see our migration file (created by EFCore, code first) contains a

RenameTable from Module to ModuleGroup

The next command is a

CreateTable Module

So the old existing Module table should be renamed to ModuleGroup and a new table also named Module should be created.
This constellation produces a

KeyNotFoundException

The error is in class SqlServerMigrationsSqlGenerator.
Below code is directly from the SqlServerMigrationsSqlGenerator class. You see that the first RenameTable operation is addressed in the switch/case block

                case RenameTableOperation renameTableOperation:
                {
                    if (temporalInformation is null)
                    {
                        temporalInformation = BuildTemporalInformationFromMigrationOperation(schema, renameTableOperation);
                    }

                    var isTemporalTable = renameTableOperation[SqlServerAnnotationNames.IsTemporal] as bool? == true;
                    if (isTemporalTable)
                    {
                        DisableVersioning(
                            tableName,
                            schema,
                            temporalInformation,
                            suppressTransaction,
                            shouldEnableVersioning: true);
                    }

                    operations.Add(operation);

                    // since table was renamed, update entry in the temporal info map
                    temporalTableInformationMap[(renameTableOperation.NewName!, renameTableOperation.NewSchema)] = temporalInformation;
                    temporalTableInformationMap.Remove((tableName, schema));

                    break;
                }

In this case block the table name is removed from dictionary temporalTableInformationMap (You can see the full code posted below).

The next foreach loop (foreach (var operation in migrationOperations)) addresses the CreateTable operation with the same name as the renamed one.

This code then produces the KeyNotFoundException because the temporalTableInformationMap does no longer contain an entry for table name "Module" in the dict "temporalTableInformationMap" because it has been removed from the dict in the prior loop (renameTable operation).

            // we are guaranteed to find entry here - we looped through all the operations earlier,
            // info missing from operations we got from the model
            // and in case of no/incomplete model we created dummy (non-temporal) entries
            var temporalInformation = temporalTableInformationMap[(tableName, rawSchema)];

So it is definitely not guaranteed that the entry is present in the temporalTableInformationMap as stated in the comment above.

Below you can find the complete snippet from class SqlServerMigrationsSqlGenerator.

        // now we do proper processing - for table operations we look at the annotations on them
        // and continuously update the stored temporal info as the table is being modified
        // for column (and other) operations we don't have annotations on them, so we look into the
        // information we stored in the initial pass and updated in when processing table ops that happened earlier
        foreach (var operation in migrationOperations)
        {
            if (operation is EnsureSchemaOperation ensureSchemaOperation)
            {
                availableSchemas.Add(ensureSchemaOperation.Name);
            }

            if (operation is not ITableMigrationOperation tableMigrationOperation)
            {
                operations.Add(operation);
                continue;
            }

            var tableName = tableMigrationOperation.Table;
            var rawSchema = tableMigrationOperation.Schema;

            var suppressTransaction = IsMemoryOptimized(operation, model, rawSchema, tableName);

            var schema = rawSchema ?? model?.GetDefaultSchema();

            // we are guaranteed to find entry here - we looped through all the operations earlier,
            // info missing from operations we got from the model
            // and in case of no/incomplete model we created dummy (non-temporal) entries
            var temporalInformation = temporalTableInformationMap[(tableName, rawSchema)];

            switch (operation)
            {
                case CreateTableOperation createTableOperation:
                {
                    // for create table we always generate new temporal information from the operation itself
                    // just in case there was a table with that name before that got deleted/renamed
                    // this shouldn't happen as we re-use existing tables rather than drop/recreate
                    // but we are being extra defensive here
                    // and also, temporal state (disabled versioning etc.) should always reset when creating a table
                    temporalInformation = BuildTemporalInformationFromMigrationOperation(schema, createTableOperation);

                    if (temporalInformation.IsTemporalTable
                        && temporalInformation.HistoryTableSchema != schema
                        && temporalInformation.HistoryTableSchema != null
                        && !availableSchemas.Contains(temporalInformation.HistoryTableSchema))
                    {
                        operations.Add(new EnsureSchemaOperation { Name = temporalInformation.HistoryTableSchema });
                        availableSchemas.Add(temporalInformation.HistoryTableSchema);
                    }

                    operations.Add(operation);

                    break;
                }

                case DropTableOperation dropTableOperation:
                {
                    var isTemporalTable = dropTableOperation[SqlServerAnnotationNames.IsTemporal] as bool? == true;
                    if (isTemporalTable)
                    {
                        // if we don't have temporal information, but we know table is temporal
                        // (based on the annotation found on the operation itself)
                        // we assume that versioning must be disabled, if we have temporal info we can check properly
                        if (temporalInformation is null || !temporalInformation.DisabledVersioning)
                        {
                            AddDisableVersioningOperation(tableName, schema, suppressTransaction);
                        }

                        if (temporalInformation is not null)
                        {
                            temporalInformation.ShouldEnableVersioning = false;
                            temporalInformation.ShouldEnablePeriod = false;
                        }

                        operations.Add(operation);

                        var historyTableName = dropTableOperation[SqlServerAnnotationNames.TemporalHistoryTableName] as string;
                        var historyTableSchema =
                            dropTableOperation[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string ?? schema;
                        var dropHistoryTableOperation = new DropTableOperation { Name = historyTableName!, Schema = historyTableSchema };
                        operations.Add(dropHistoryTableOperation);
                    }
                    else
                    {
                        operations.Add(operation);
                    }

                    // we removed the table, so we no longer need it's temporal information
                    // there will be no more operations involving this table
                    temporalTableInformationMap.Remove((tableName, schema));

                    break;
                }

                case RenameTableOperation renameTableOperation:
                {
                    if (temporalInformation is null)
                    {
                        temporalInformation = BuildTemporalInformationFromMigrationOperation(schema, renameTableOperation);
                    }

                    var isTemporalTable = renameTableOperation[SqlServerAnnotationNames.IsTemporal] as bool? == true;
                    if (isTemporalTable)
                    {
                        DisableVersioning(
                            tableName,
                            schema,
                            temporalInformation,
                            suppressTransaction,
                            shouldEnableVersioning: true);
                    }

                    operations.Add(operation);

                    // since table was renamed, update entry in the temporal info map
                    temporalTableInformationMap[(renameTableOperation.NewName!, renameTableOperation.NewSchema)] = temporalInformation;
                    temporalTableInformationMap.Remove((tableName, schema));

                    break;
                }

Please let me know if you need additional information.


EF Core version: 9.0.0
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET8.0
Operating system: Windows
IDE: Visual Studio 2022 17.12.3

@roji
Copy link
Member

roji commented Dec 12, 2024

/cc @maumar, probable dup of #35162.

@maumar
Copy link
Contributor

maumar commented Dec 13, 2024

@roji yes it's related. I've already encountered this case when working on #35162. I should be able to fix them both at the same time.

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

No branches or pull requests

3 participants