Add support for defining included column for indexes in SQL server #12266
Conversation
|
|
|
I've used
|
|
Rebased onto latest dev. One thing I think is worth noting is that Other than that I have not found any further issues, so this should be ready for review. |
Kukkimonsuta
changed the title
| @@ -1896,7 +1896,7 @@ protected virtual bool HasDifferences([NotNull] IEnumerable<IAnnotation> source, | |||
|
|
|||
| foreach (var annotation in source) | |||
| { | |||
| var index = unmatched.FindIndex(a => a.Name == annotation.Name && Equals(a.Value, annotation.Value)); | |||
| var index = unmatched.FindIndex(a => a.Name == annotation.Name && AnnotationValueEquals(a.Value, annotation.Value)); | |||
There was a problem hiding this comment.
Just call StructuralComparisons.StructuralEqualityComparer.Equals
This was addressed here f947625 Let me know if I should rebase/squash changes |
This commit adds `ForSqlServerInclude` extension to `IndexBuilder` allowing defining columns to be included in a index on sql server. Generic version of `IndexBuilder` together with its extensions is introduced to support expression parameter. Breaking change is introduced in `SqlServer` adding a property to interface - `ISqlServerIndexAnnotations.IncludeProperties`. `MigrationsModelDiffer` now uses structural comparison for annotations to support arrays as annotation value. Fixes #4846
|
Rebased and squashed. |
|
@Kukkimonsuta Thanks for your contribution! |
This PR adds
ForSqlServerIncludeextension toIndexBuilderallowing defining columns to be included in a index on sql server.To support expression parameter a generic version of
IndexBuilderis introduced in main EF assembly, however to avoid breaking changes (as requested in #4846 (comment)) this type is available only with new extensionForSqlServerHasIndexfor the time being.There is a breaking change in
EFCore.SqlServeradding a property to interface -ISqlServerIndexAnnotations.Include, which might be ok since it's maybe not really meant to be implemented by others? @ajcvickersI'm still investigating possible problems with migrations, so this is not yet ready.
Fixes #4846