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: stackoverflow for may be invalid relationship via fluent api #9478

Closed
smitpatel opened this issue Aug 19, 2017 · 17 comments
Closed
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. regression type-bug
Milestone

Comments

@smitpatel
Copy link
Contributor

Model

public class Blog
{
    public int Id { get; set; }
    public Blog MyBlog { get; set; }
    public Blog InverseBlog { get; set; }
}

// Write following in OnModelCreating method
modelBuilder.Entity<Blog>().HasOne(e => e.MyBlog).WithOne(e => e.InverseBlog).HasForeignKey<Blog>(e => e.Id);

Watch it 🔥

@smitpatel
Copy link
Contributor Author

smitpatel commented Aug 19, 2017

Throw in this line in OnModelCreating it ignores the relationship and creates database without FK.
modelBuilder.Entity<Blog>().Property(e => e.Id).HasColumnName("id");

@smitpatel
Copy link
Contributor Author

This worked in 1.1.2 packages creating a self-ref fk. (defining such relationship is possible in SqlServer).
It initially gave error on cascade but setting behavior to restrict worked.

      CREATE TABLE [Blogs] (
          [Id] int NOT NULL,
          CONSTRAINT [PK_Blogs] PRIMARY KEY ([Id]),
          CONSTRAINT [FK_Blogs_Blogs_Id] FOREIGN KEY ([Id]) REFERENCES [Blogs] ([Id]) ON DELETE NO ACTION
      );

@AndriySvyryd
Copy link
Member

Impact: Self-referencing PK-to-PK relationships cause stack overflow.
Risk: Low, the fix only affects this scenario

@AlanMacdonald
Copy link

This is a big issue for us. Is there any likely release date for 2.0.1? Thanks

@Nyami
Copy link

Nyami commented Sep 5, 2017

This is also a blocker for us, interested to know the timescales for 2.0.1 too...
Should also note that this also appears to affect data annotated models..

@ajcvickers
Copy link
Contributor

Tentative schedule for this fix is early October, but that could change.

@tandradeflorencio
Copy link

Waiting a solution too. Tks.

@dabasejumper
Copy link

That had to fail a unit test somewhere!

@ajcvickers
Copy link
Contributor

I think the tests missed this because a PK-to-PK self referencing relationship doesn't seem to have any point. It always points to itself and can never change. For people hitting this, it would be interesting to know if there is a reason that your databases contain these relationships?

@dabasejumper
Copy link

dabasejumper commented Sep 11, 2017

I don't even have anything that crazy and I'm getting the same error... In my example a person should have a TimeZone associated with them. Here are two tables I have in the DB.

CREATE TABLE [dbo].[TimeZone]
(
    [TimeZoneId] SMALLINT NOT NULL PRIMARY KEY IDENTITY, 
    [Code] VARCHAR(5) NOT NULL, 
    [Name] VARCHAR(150) NOT NULL, 
    [Description] VARCHAR(150) NOT NULL
)

CREATE TABLE [dbo].[Person] (
    [Id]               INT          NOT NULL,
    [FirstName]        VARCHAR (50) NULL,
    [MiddleName]       VARCHAR (50) NULL,
    [LastName]         VARCHAR (50) NULL,
    [PersonPrefixId]   SMALLINT          NULL,
    [Suffix]           VARCHAR (20) NULL,
    [Title]            VARCHAR (50) NULL,
	[TimeZoneId] SMALLINT NOT NULL DEFAULT 13, 
    [AppUserId] UNIQUEIDENTIFIER NULL,
    [CreatedByUserId] UNIQUEIDENTIFIER NOT NULL, 
    [CreatedDate]      DATETIME     CONSTRAINT [DF_Person_CreatedDate] DEFAULT (getutcdate()) NOT NULL,
    [LastModifiedByUserId] UNIQUEIDENTIFIER NOT NULL, 
    [LastModifiedDate] DATETIME     CONSTRAINT [DF_Person_LastModifiedDate] DEFAULT (getutcdate()) NOT NULL,
    CONSTRAINT [PK_Person] PRIMARY KEY CLUSTERED ([Id] ASC),
    CONSTRAINT [FK_Person_is_a_Party] FOREIGN KEY ([Id]) REFERENCES [dbo].[Party] ([Id]),
    CONSTRAINT [FK_Person_PersonPrefixId] FOREIGN KEY ([PersonPrefixId]) REFERENCES [dbo].[PersonPrefix] ([Id]), 
    CONSTRAINT [FK_Person_CreatedByUserId] FOREIGN KEY ([CreatedByUserId]) REFERENCES [dbo].[AppUser]([Id]), 
    CONSTRAINT [FK_Person_LastModifiedByUserId] FOREIGN KEY ([LastModifiedByUserId]) REFERENCES [dbo].[AppUser]([Id]), 
    CONSTRAINT [FK_Person_AppUserId] FOREIGN KEY ([AppUserId]) REFERENCES [dbo].[AppUser]([Id]),
	CONSTRAINT [FK_Person_TimeZoneId] FOREIGN KEY ([TimeZoneId]) REFERENCES [dbo].[TimeZone] ([TimeZoneId]),
)

After the database is created from the project.. I use the following scaffolding command to generate my classes.

Scaffold-DbContext -Verbose "Server=BLAH;Database=OC;Trusted_Connection=True;" Microsoft.EntityFrameworkCore.SqlServer -Schemas "dbo" -Force

Here are the classes that are generated by the scaffolding:

public partial class Person
    {
        public TimeZone TimeZone { get; set; }

    }

public partial class TimeZone
    {
        public TimeZone()
        {
            Person = new HashSet<Person>();
        }

        public short TimeZoneId { get; set; }
        public string Code { get; set; }
        public string Name { get; set; }
        public string Description { get; set; }

        public ICollection<Person> Person { get; set; }
    }

Then, with a simple console application running it breaks on accessing the data....
var _ctx = serviceProvider.GetService();
var tzList = _ctx.TimeZone.ToList();

Exception : System.StackOverflowException
Message Evaluation of method System.Exception.get_Message requires calling method System.Reflection.RuntimeMethodInfo.CreateDelegate, which cannot be called in this context.

In the EF file Annotatable.cs Line 130
Annotation annotation;
return _annotations.Value.TryGetValue(name, out annotation)
? annotation
: null;

Let me know, I'm sure I can put together a smaller example project if you need me to.

@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Sep 11, 2017
@ajcvickers
Copy link
Contributor

@AndriySvyryd Can you verify the code above is the same root cause and is fixed in 2.0.1?

@AndriySvyryd
Copy link
Member

@dabasejumper Could you share a repro with a complete model? The original DB schema is not needed.

@ajcvickers
Copy link
Contributor

Hi @AlanMacdonald, @Nyami, @dabasejumper, @tandradeflorencio. We are gathering information on the use of EF Core pre-release builds. You reported this issue shortly after the release of 2.0.0 RTM. It would be really helpful if you could let us know:

  • Did you consider testing your code against the pre-release builds?
  • Was there anything blocking you from using pre-release builds?
  • What do you think could make it easier for you to use pre-release builds in the future?

Thanks in advance for any feedback. Hopefully this will help us to increase the value of pre-release builds going forward.

@AndriySvyryd
Copy link
Member

Workaround: If you are scaffolding from an existing database you can use the query from #9462 (comment) to find the redundant FKs that cause this issue. Then either drop them or comment out the corresponding navigation properties.

@dabasejumper
Copy link

dabasejumper commented Sep 12, 2017

@AndriySvyryd @ajcvickers I could not reproduce this issue with a simple two table solution. I tried last night. It wasn't even an issue with the EF/Linq query that I was trying to execute. It was further down the model.

So sql from #9462 (comment) fixed my issue. It was NOT with the two tables I was looking it. It unveiled a typo that I had made in one of the table designers of in my DB project.

It was as described above... A self referencing FK.... Was supposed to be DataLoadBatch instead of just DataLoad :)

Thank you for your feedback helping me thru this issue... Although, this issue probably needs a better error message when it is encountered.

Thanks for everything... Mike

@Eilon
Copy link
Member

Eilon commented Sep 18, 2017

This patch bug is approved for the 2.0.x patch. Please send a PR to the feature/2.0.1 branch and get it reviewed and merged. When we have the rel/2.0.1 branches ready please port the commit to that branch.

@Eilon
Copy link
Member

Eilon commented Oct 23, 2017

Hi, we have a public test feed that you can use to try out the ASP.NET/EF Core 2.0.3 patch!

To try out the pre-release patch, please refer to the following guide:

We are looking for feedback on this patch. We'd like to know if you have any issues with this patch by updating your apps and libraries to the latest packages and seeing if it fixes the issues you've had, or if it introduces any new issues. If you have any issues or questions, please reply on this issue to let us know as soon as possible.

Thanks,
Eilon

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. regression type-bug
Projects
None yet
Development

No branches or pull requests

8 participants