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

Add test coverage for index recreation after altering computed columns #26656

Closed
benstevens48 opened this issue Nov 12, 2021 · 1 comment · Fixed by #26657
Closed

Add test coverage for index recreation after altering computed columns #26656

benstevens48 opened this issue Nov 12, 2021 · 1 comment · Fixed by #26657
Assignees
Labels
area-migrations area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@benstevens48
Copy link

Description

If an index exists on a column before a migration and an identical index should exist after a migration, then the index is not added during the migration. This seems to make sense but it fails to take into account that an index on a column will be automatically dropped if the column is dropped. Certain migrations will cause a column to be dropped and re-added, such as changing the SQL of a computed column. You can test it using the code below.

Code

See also this sample solution

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using System;
using System.ComponentModel.DataAnnotations;
using System.ComponentModel.DataAnnotations.Schema;

namespace EfTest.DbModels {
    [Table("ProductItems")]
    public class ProductDbItem {
        [Key]
        [Column("Id")]
        public Guid Id { get; set; }
        [Required]
        [Column("DisplayName")]
        public string DisplayName { get; set; }
        [Column("TestCol")]
        public string TestCol { get; set; }
    }

    public class ProductEntityTypeConfiguration : IEntityTypeConfiguration<ProductDbItem> {
        public void Configure(EntityTypeBuilder<ProductDbItem> builder) {
            builder.Property(item => item.TestCol).HasComputedColumnSql("coalesce(\"DisplayName\", '')", true);
            builder.HasIndex(item => item.TestCol);
        }
    }
}

Generate an initial migration and script with the above model.

Now modify the HasComputedColumnSql to something else, e.g.

"coalesce(\"DisplayName\", '') || 'Test'"

Generate a migration from the previous migration.

The initial migration code loos like this

CREATE TABLE IF NOT EXISTS "__EFMigrationsHistory" (
    "MigrationId" character varying(150) NOT NULL,
    "ProductVersion" character varying(32) NOT NULL,
    CONSTRAINT "PK___EFMigrationsHistory" PRIMARY KEY ("MigrationId")
);

START TRANSACTION;

CREATE TABLE "ProductItems" (
    "Id" uuid NOT NULL,
    "DisplayName" text NOT NULL,
    "TestCol" text GENERATED ALWAYS AS (coalesce("DisplayName", '')) STORED,
    CONSTRAINT "PK_ProductItems" PRIMARY KEY ("Id")
);

CREATE INDEX "IX_ProductItems_TestCol" ON "ProductItems" ("TestCol");

INSERT INTO "__EFMigrationsHistory" ("MigrationId", "ProductVersion")
VALUES ('20211112165652_InitialMigration', '5.0.11');

COMMIT;

The next migration looks like this

START TRANSACTION;

ALTER TABLE "ProductItems" DROP COLUMN "TestCol";

ALTER TABLE "ProductItems" ADD "TestCol" text GENERATED ALWAYS AS (coalesce("DisplayName", '') || 'Test') STORED;

INSERT INTO "__EFMigrationsHistory" ("MigrationId", "ProductVersion")
VALUES ('20211112165736_ChangeCol', '5.0.11');

COMMIT;

Observe that the index is not re-added even though it will be dropped when the column is dropped.

Expected behavior

The index should be added after the migration.

There are a number of ways this could be done. One way would be to always drop indexes on columns that are dropped beforehand, then add the indexes again afterwards. (Dropping the index or checking if the index exists or calculating if the index will be automatically dropped is needed because I think the index may not be automatically dropped if it involves other columns).

Provider and version information

EF Core version: 5.0.11
Database provider: Any I assume but tested using Postgres
Target framework: NET 5.0
Operating system: Windows 10 21H1
IDE: Visual Studio 2019 16.11

@roji
Copy link
Member

roji commented Nov 12, 2021

This does work correctly in SQL Server which has specific code to drop and recreate the indexes, but the PostgreSQL provider doesn't this. We're missing migration test coverage for this, will add that.

Opened npgsql/efcore.pg#2094 to track on the Npgsql side.

/cc @lauxjpn in case the Pomelo provider also needs this.

@roji roji added the area-test label Nov 12, 2021
@roji roji self-assigned this Nov 12, 2021
roji added a commit to roji/efcore that referenced this issue Nov 12, 2021
@roji roji changed the title Migrations bug - index not re-added after migration which drops and re-adds column Add test coverage for index recreation after altering computed columns Nov 12, 2021
roji added a commit that referenced this issue Nov 13, 2021
@ajcvickers ajcvickers added this to the 7.0.0 milestone Nov 16, 2021
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. area-migrations labels Nov 16, 2021
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-preview1 Feb 14, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-preview1, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants