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

ModelBuilder: QueryTypeBuilder.ToView continuously generates EnsureSchema in migrations. #14195

Closed
sergeitemkin opened this issue Dec 17, 2018 · 8 comments · Fixed by #17101
Closed
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@sergeitemkin
Copy link

When binding a QueryType to a view or table using QueryTypeBuilder.ToView and specifying a schema, an EnsureSchema statement is created for the QueryType in every subsequent migration. It doesn't break anything, but it does get a little bit tedious to keep picking the rogue EnsureSchemas out of migration scripts.

Steps to reproduce

  1. Create a DbContext with a QueryType.
public class SomeType
{
    public string Column1 { get; set; }
}

public class SampleContext : DbContext
{
    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Query<SomeType>(builder =>
        {
            builder.ToView(schema: "SomeSchema", name: "Things");
        });
    }
}
  1. Run dotnet ef migrations add Test
    • I'm expecting the model snapshot to have some reference to EnsureSchema or to maybe even just have an empty migration since ensuring that the schema exists doesn't really help if the query is relying on existence of some SQL table or view that isn't being auto-generated by the migration
  2. Run dotnet ef migrations add TestAgain
    • Both migrations contain an EnsureSchema statement
    protected override void Up(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.EnsureSchema(
            name: "SomeSchema");
    }

EfCoreViewWithSchema.zip

Further technical details

EF Core version: 2.2.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10
IDE: Visual Studio 2019 16.0.0 Preview 1.1

@smitpatel
Copy link
Contributor

https://github.com/aspnet/EntityFrameworkCore/blob/af13b52bfc390346551926ca56d5b0703c1eb0e7/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs#L1955-L1959

GetSchemas function needs to ignore QueryTypes when getting schemas since migrations don't deal with query types.

@smitpatel smitpatel added type-bug help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. labels Dec 17, 2018
@ajcvickers ajcvickers added this to the 3.0.0 milestone Dec 19, 2018
@Marusyk
Copy link
Member

Marusyk commented Jan 9, 2019

Hello, I would try to prepare PR for this, if nobody is against it

@ajcvickers
Copy link
Contributor

@Marusyk Sure, go for it.

@Marusyk
Copy link
Member

Marusyk commented Jan 23, 2019

I don't understand how to ignore QueryTypes in GetSchemas (it's not so easy as I thought). So, it would be better to assign this task to someone else. Sorry. It is up-for-grabs again

@bricelam
Copy link
Contributor

No worries, @Marusyk; We'll get to it sometime during the 3.0.0 release.

@divega divega added good first issue This issue should be relatively straightforward to fix. help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. and removed help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. good first issue This issue should be relatively straightforward to fix. labels May 31, 2019
@bricelam bricelam added the good first issue This issue should be relatively straightforward to fix. label May 31, 2019
@bricelam bricelam added the verify-fixed This issue is likely fixed in new query pipeline. label Jun 20, 2019
@HaoK HaoK assigned HaoK and unassigned bricelam Jul 2, 2019
@HaoK
Copy link
Member

HaoK commented Jul 3, 2019

Verified that both migrations do still contain EnsureSchema.

Steps I did: (VS 2019 Enterprise 16.1.3)

  1. File New MVC (Individual auth to pull in EF) (3.0.0-preview6.19304.10)
  2. Copy SampleContext into app (and configure it to use DefaultConnection SqlServer)
  3. In Package Manager console run: Add-Migration test -Context SampleContext
  4. Add-Migration testAgain -Context SampleContext

Both still contain EnsureSchema

@ajcvickers this doesn't appear fixed, I'm not sure what labels you want me to set for this, so I will just unassign this bug for now

@HaoK HaoK removed their assignment Jul 3, 2019
@HaoK
Copy link
Member

HaoK commented Jul 3, 2019

Note: I also tried fixing the obsolete warnings and used:

            modelBuilder.Entity<SomeType>().HasNoKey().ToTable(schema: "SomeSchema", name: "Things");

Which still has EnsureSchema in both migrations

@smitpatel smitpatel removed this from the 3.0.0 milestone Jul 3, 2019
@smitpatel smitpatel removed the verify-fixed This issue is likely fixed in new query pipeline. label Jul 3, 2019
@bricelam
Copy link
Contributor

bricelam commented Jul 3, 2019

Thanks for checking. I have another related issue that may also fix this one. Applied the label too soon. 😉

@ajcvickers ajcvickers added this to the 3.0.0 milestone Jul 5, 2019
@ajcvickers ajcvickers removed the help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. label Aug 5, 2019
@bricelam bricelam added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed good first issue This issue should be relatively straightforward to fix. labels Aug 12, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview9 Aug 21, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview9, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants