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.

Sign up for GitHub

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

Jump to bottom

ForeignKeyAttribute on skip navigations fails to remove by-convention join table from model #27990

Closed
janschreier opened this issue May 10, 2022 · 5 comments · Fixed by #32640
Closed

ForeignKeyAttribute on skip navigations fails to remove by-convention join table from model #27990

janschreier opened this issue May 10, 2022 · 5 comments · Fixed by #32640
Labels
area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-bug
Milestone
9.0.0

Comments

@janschreier
Copy link

With code created by dbScaffold (using global tool 6.0.4) in EF Core 6.0.4 I get the following error:

Exception thrown: 'Microsoft.Data.Sqlite.SqliteException' in Microsoft.EntityFrameworkCore.Relational.dll
An unhandled exception of type 'Microsoft.Data.Sqlite.SqliteException' occurred in Microsoft.EntityFrameworkCore.Relational.dll
SQLite Error 1: 'table "MotorartMotorbauart" already exists'.

I think the error must exist for at least some time, but I can't tell exactly which version introduced it.

Here's a sample program creating that error: https://github.com/janschreier/EfCoreInMemoryBug
the dbContext code-is from a larger project and i tried to strip it down as much as possible while still receiving the same error. So it looks a bit akward.

OnModelCreating() is also called twice which is unexpected to me but then I never had a breakpoint on it so I'm not sure if that's okay or not

EF Core version: 6.0.4
Database provider: Microsoft.EntityFrameworkCore.Sqlite
Target framework: net6.0.4
Operating system: win 10x64
IDE: Visual Studio 2022 17.1.6
@AndriySvyryd
Copy link
Member

Probably a duplicate of #25797

@AndriySvyryd AndriySvyryd added this to the 7.0.0 milestone May 10, 2022
@ajcvickers ajcvickers added propose-punt punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. and removed propose-punt labels Jul 6, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0, Backlog Jul 7, 2022
@bricelam
Copy link
Contributor

bricelam commented Apr 4, 2023

@ajcvickers Do you think this was fixed by PR #29794?

@ajcvickers
Copy link
Member

@bricelam Still throws with 8.0 preview 2

@bricelam bricelam removed their assignment Jul 8, 2023
@christianlevesque
Copy link

We were having a similar problem at work when setting up a unit test database, and we thought maybe it was related to this issue. However, in the process of diagnosing our error, we also figured out what's happening with the reproduction code. I know it's old, but it's still open, so I figured I would drop a quick comment.

The reproduction code is configuring the relationships twice - once in DbContext.OnModelCreating() and again in the models using attributes. I submitted a PR back into the reproduction project showing that by removing the attribute configuration, the code runs as expected.

ajcvickers added a commit that referenced this issue Dec 18, 2023
@ajcvickers
Don't keep join table around because a ForeignKeyAttribute used to re…
0a2246c 
…ference it

Fixes #27990

The issue here was a skip navigation with a `ForeignKeyAttribute`. At first I thought that this was not valid, but it turns out we decided that this configuration specified the name of the FK property in the join table.

However, this by-convention join type was then discarded and replaced with an explicitly configured join type. This normally removes the old join type from the model, but this doesn't happen here because the `ForeignKeyAttribute` resulted in the join type having a "data annotation" configuration source. This is not really correct; the FK attribute just says that the FK properties should have a given name, not anything about the type, so the entity type configuration source should not be changed here.

This allows the by-convention join type to be removed like it should, and hence we don't have two join tables mapped that differ only by case.
ajcvickers added a commit that referenced this issue Dec 18, 2023
@ajcvickers
Don't keep join table around because a ForeignKeyAttribute used to re…
9af6362 
…ference it

Fixes #27990

The issue here was a skip navigation with a `ForeignKeyAttribute`. At first I thought that this was not valid, but it turns out we decided that this configuration specified the name of the FK property in the join table.

However, this by-convention join type was then discarded and replaced with an explicitly configured join type. This normally removes the old join type from the model, but this doesn't happen here because the `ForeignKeyAttribute` resulted in the join type having a "data annotation" configuration source. This is not really correct; the FK attribute just says that the FK properties should have a given name, not anything about the type, so the entity type configuration source should not be changed here.

This allows the by-convention join type to be removed like it should, and hence we don't have two join tables mapped that differ only by case.
@ajcvickers ajcvickers mentioned this issue Dec 18, 2023
Closed
@ajcvickers ajcvickers self-assigned this Dec 18, 2023
@ajcvickers ajcvickers added area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed area-sqlite labels Dec 18, 2023
@ajcvickers ajcvickers modified the milestones: Backlog, 9.0.0 Dec 18, 2023
@ajcvickers ajcvickers changed the title SQLite InMemory EnsureCreated() throws SqliteException (table already exists) ForeignKeyAttribute on skip navigations fails to remove by-convention join table from model Dec 18, 2023
@ajcvickers
Copy link
Member

@AndriySvyryd Happy for you to take this over; I almost gave up when I realized it had nothing to do with SQLite. :-D The tests, at least, should be helpful.

AndriySvyryd added a commit that referenced this issue Dec 19, 2023
@AndriySvyryd
Don't keep join entity type around because a ForeignKeyAttribute used…
227c970 
… to reference it. Instead, remove it and reattach the configuration.

Also, reuse the FKs that already were configured instead of creating new ones.

Fixes #27990
Fixes #26555
@AndriySvyryd AndriySvyryd mentioned this issue Dec 19, 2023
Merged
AndriySvyryd added a commit that referenced this issue Dec 19, 2023
@AndriySvyryd
Don't keep join entity type around because a ForeignKeyAttribute used…
cf303c8 
… to reference it. Instead, remove it and reattach the configuration.

Also, reuse the FKs that already were configured instead of creating new ones.

Fixes #27990
Fixes #26555
AndriySvyryd added a commit that referenced this issue Dec 19, 2023
@AndriySvyryd
Don't keep join entity type around because a ForeignKeyAttribute used…
a36839f 
… to reference it. Instead, remove it and reattach the configuration. (#32640)

Also, reuse the FKs that already were configured instead of creating new ones.

Fixes #27990
Fixes #26555
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview1 Jan 31, 2024
@roji roji modified the milestones: 9.0.0-preview1, 9.0.0 Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-bug
Projects
None yet
Milestone
9.0.0
6 participants
@bricelam @ajcvickers @roji @janschreier @AndriySvyryd @christianlevesque