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

Don't identify dictionary indexer as NRT #22068

Merged
merged 2 commits into from
Aug 18, 2020
Merged

Conversation

roji
Copy link
Member

@roji roji commented Aug 14, 2020

We currently don't calculate nullability for generic properties, since that's a complex task (dotnet/runtime#29723 is supposed to add this capability to the reflection API at some point). This interferes with the ability to use Dictionary for property bag, since it's a generic type and its indexer is incorrectly identified as non-nullable (the actual nullability of the generic type argument isn't taken into account).

The general inability to handle generic properties hasn't proved problematic - it's been this way since EF Core 3.0 and we've received no user complaints. This is a specific fix to recognize the indexer on Dictionary and not identify it as non-nullable. If this is OK to merge, I'll document the general limitation in our doc page on nullability, and open a separate issue for handling generic properties.

Fixes #20718

@roji roji requested a review from a team August 14, 2020 21:06
@roji roji force-pushed the DictionaryIndexerIsNullable branch from 8979c3d to c50f544 Compare August 15, 2020 15:22
@smitpatel
Copy link
Member

System.AggregateException : One or more errors occurred. (Introducing FOREIGN KEY constraint 'FK_JoinTwoSelfShared_EntityTwos_RightId' on table 'JoinTwoSelfShared' may cause cycles or multiple cascade paths. Specify ON DELETE NO ACTION or ON UPDATE NO ACTION, or modify other FOREIGN KEY constraints.\r\nCould not create constraint or index. See previous errors.) (The following constructor parameters did not have matching fixture data: ManyToManyLoadSqlServerFixture fixture)\r\n---- Microsoft.Data.SqlClient.SqlException : Introducing FOREIGN KEY constraint 'FK_JoinTwoSelfShared_EntityTwos_RightId' on table 'JoinTwoSelfShared' may cause cycles or multiple cascade paths. Specify ON DELETE NO ACTION or ON UPDATE NO ACTION, or modify other FOREIGN KEY constraints.\r\nCould not create constraint or index. See previous errors.\r\n---- The following constructor parameters did not have matching fixture data: ManyToManyLoadSqlServerFixture fixture

@AndriySvyryd - Shouldn't the convention take care of this?

@AndriySvyryd
Copy link
Member

Yes, it should. It might be missing a trigger

Copy link
Member

@smitpatel smitpatel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving Andriy's changes

@AndriySvyryd AndriySvyryd changed the base branch from main to release/5.0 August 17, 2020 22:05
@roji roji merged commit c607441 into release/5.0 Aug 18, 2020
@roji roji deleted the DictionaryIndexerIsNullable branch August 18, 2020 06:29
@roji
Copy link
Member Author

roji commented Aug 18, 2020

Thanks @AndriySvyryd and @smitpatel

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

Successfully merging this pull request may close these issues.

IndexerProperty can't have a null value in one of its columns
3 participants