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

A unique index only means an alternate key if it is the principal end of an FK #8089

Closed
wants to merge 1 commit into from

Conversation

lajones
Copy link
Contributor

@lajones lajones commented Apr 6, 2017

Fixes issue #7956.

Previously a unique index implied that the properties on which that index is based would be set as required even if the underlying columns were nullable. Now we only do this if the properties are the principal end of a foreign key. This prevents columns which just happen to use a unique index but have nothing to do with a FK being set to required when this is not necessary.

@@ -168,4 +168,7 @@
<data name="MissingTable" xml:space="preserve">
<value>Unable to find a table in the database matching the selected table {table}.</value>
</data>
<data name="ForeignKeyPrincipalEndContainsNullableColumns" xml:space="preserve">
<value>The principal end of the foreign key '{foreignKeyName}' contains the following nullable columns '{columnNames}'. Entity Framework requires the properties representing those columns to be non-nullable.</value>
</data>
Copy link
Contributor Author

@lajones lajones Apr 6, 2017

Choose a reason for hiding this comment

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

Forgot to mention the index here. I'll make it something like "The principal end of the foreign key '{foreignKeyName}' is supported by the unique index '{indexName}' and contains the following nullable columns '{columnNames}'...." and check in.

@lajones lajones added this to the 2.0.0 milestone Apr 6, 2017
@lajones lajones added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. providers-beware labels Apr 6, 2017
@lajones
Copy link
Contributor Author

lajones commented Apr 6, 2017

Checked in with commit 439f510.

@lajones lajones closed this Apr 6, 2017
@lajones
Copy link
Contributor Author

lajones commented Apr 6, 2017

Added providers-beware. Don't think this will have a big effect on providers, but the change affects, in some cases, the model that RelationalScaffoldingModelFactory produces. Providers inherit from this - so this is a heads-up for those changes.

@smitpatel
Copy link
Member

We should avoid having PRs without proper merge. If we merge PR with fast forward then git marks it as merged PR and also stores reference to it while looking at commit. This makes it much easier to come back to PR from a commit at later stage if needed.

@lajones lajones deleted the 170404_7956_UniqueIndexIsNotAlternateKey_01 branch April 7, 2017 14:46
@lajones
Copy link
Contributor Author

lajones commented Apr 7, 2017

@smitpatel we should talk about this offline. I just merged from the command-line (I did not specify the ff flag) and everything looks normal to me on gitk.

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. providers-beware
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants