Don't treat a filtered index as covering a foreign key index

src/EFCore.Relational/EFCore.Relational.baseline.json

@@ -12486,6 +12486,22 @@
         }
       ]
     },
+    {
+      "Type": "class Microsoft.EntityFrameworkCore.Metadata.Conventions.RelationalForeignKeyIndexConvention : Microsoft.EntityFrameworkCore.Metadata.Conventions.ForeignKeyIndexConvention, Microsoft.EntityFrameworkCore.Metadata.Conventions.IIndexAnnotationChangedConvention, Microsoft.EntityFrameworkCore.Metadata.Conventions.IConvention",
+      "Methods": [
+        {
+          "Member": "RelationalForeignKeyIndexConvention(Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure.ProviderConventionSetBuilderDependencies dependencies, Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure.RelationalConventionSetBuilderDependencies relationalDependencies);"
+        },
+        {
+          "Member": "virtual void ProcessIndexAnnotationChanged(Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionIndexBuilder indexBuilder, string name, Microsoft.EntityFrameworkCore.Metadata.IConventionAnnotation? annotation, Microsoft.EntityFrameworkCore.Metadata.IConventionAnnotation? oldAnnotation, Microsoft.EntityFrameworkCore.Metadata.Conventions.IConventionContext<Microsoft.EntityFrameworkCore.Metadata.IConventionAnnotation> context);"
+        }
+      ],
+      "Properties": [
+        {
+          "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure.RelationalConventionSetBuilderDependencies RelationalDependencies { get; }"
+        }
+      ]
+    },
     {
       "Type": "abstract class Microsoft.EntityFrameworkCore.Storage.RelationalGeometryTypeMapping<TGeometry, TProvider> : Microsoft.EntityFrameworkCore.Storage.RelationalTypeMapping",
       "Methods": [

src/EFCore.Relational/Metadata/Conventions/Infrastructure/RelationalConventionSetBuilder.cs

@@ -80,6 +80,9 @@ public override ConventionSet CreateConventionSet()
         conventionSet.Replace<ValueGenerationConvention>(
             new RelationalValueGenerationConvention(Dependencies, RelationalDependencies));
 
+        conventionSet.Replace<ForeignKeyIndexConvention>(
+            new RelationalForeignKeyIndexConvention(Dependencies, RelationalDependencies));
+
         conventionSet.Replace<KeyDiscoveryConvention>(
             new RelationalKeyDiscoveryConvention(Dependencies, RelationalDependencies));
 

src/EFCore.Relational/Metadata/Conventions/RelationalForeignKeyIndexConvention.cs

@@ -0,0 +1,99 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
+
+/// <summary>
+///     A relational-specific convention inheriting from <see cref="ForeignKeyIndexConvention" /> that takes index filters
+///     into account.
+/// </summary>
+/// <remarks>
+///     <para>
+///         An index with a filter only includes some of the rows in the table, so it cannot be used in place of the
+///         index on the foreign key properties that would otherwise be created by convention. The exception is a filter that
+///         was set by convention, such as the filter that SQL Server uses to exclude rows with null values from unique indexes.
+///         These filters are not considered when deciding whether an index is needed. Strictly, such a filter can also exclude rows
+///         that a lookup on a prefix of the indexed columns needs, but treating it as covering keeps the behavior that models
+///         without explicit filters have always had, and so avoids adding indexes to existing models.
+///     </para>
+///     <para>
+///         See <see href="https://aka.ms/efcore-docs-conventions">Model building conventions</see> for more information and examples.
+///     </para>
+/// </remarks>
+public class RelationalForeignKeyIndexConvention : ForeignKeyIndexConvention, IIndexAnnotationChangedConvention
+{
+    /// <summary>
+    ///     Creates a new instance of <see cref="RelationalForeignKeyIndexConvention" />.
+    /// </summary>
+    /// <param name="dependencies">Parameter object containing dependencies for this convention.</param>
+    /// <param name="relationalDependencies"> Parameter object containing relational dependencies for this convention.</param>
+    public RelationalForeignKeyIndexConvention(
+        ProviderConventionSetBuilderDependencies dependencies,
+        RelationalConventionSetBuilderDependencies relationalDependencies)
+        : base(dependencies)
+        => RelationalDependencies = relationalDependencies;
+
+    /// <summary>
+    ///     Relational provider-specific dependencies for this service.
+    /// </summary>
+    protected virtual RelationalConventionSetBuilderDependencies RelationalDependencies { get; }
+
+    /// <summary>
+    ///     Called after an annotation is changed on an index.
+    /// </summary>
+    /// <param name="indexBuilder">The builder for the index.</param>
+    /// <param name="name">The annotation name.</param>
+    /// <param name="annotation">The new annotation.</param>
+    /// <param name="oldAnnotation">The old annotation.</param>
+    /// <param name="context">Additional information associated with convention execution.</param>
+    public virtual void ProcessIndexAnnotationChanged(
+        IConventionIndexBuilder indexBuilder,
+        string name,
+        IConventionAnnotation? annotation,
+        IConventionAnnotation? oldAnnotation,
+        IConventionContext<IConventionAnnotation> context)
+    {
+        if (name == RelationalAnnotationNames.Filter)
+        {
+            ProcessIndexCoverageChanged(indexBuilder.Metadata);
+        }
+    }
+
+    /// <summary>
+    ///     Called when a model is being finalized.
+    /// </summary>
+    /// <remarks>
+    ///     Upgrading a filter from being set by convention to being set explicitly, without changing its value, is not reported
+    ///     as an annotation change. This makes sure that foreign key indexes are created for any index that is partial at this point.
+    /// </remarks>
+    /// <param name="modelBuilder">The builder for the model.</param>
+    /// <param name="context">Additional information associated with convention execution.</param>
+    public override void ProcessModelFinalizing(
+        IConventionModelBuilder modelBuilder,
+        IConventionContext<IConventionModelBuilder> context)
+    {
+        foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
+        {
+            foreach (var index in entityType.GetDeclaredIndexes().ToList())
+            {
+                if (index.IsInModel
+                    && IsPartial(index))
+                {
+                    ProcessIndexCoverageChanged(index);
+                }
+            }
+        }
+
+        base.ProcessModelFinalizing(modelBuilder, context);
+    }
+
+    /// <inheritdoc />
+    /// <remarks>
+    ///     An index is partial if it has a filter that was not set by convention. Filters set by convention, such as the filter
+    ///     used by SQL Server to exclude rows with null values from unique indexes, are ignored. The filter is read from the index
+    ///     itself, so a filter that only applies to a specific table because of table sharing is not taken into account.
+    /// </remarks>
+    protected override bool IsPartial(IConventionIndex index)
+        => index.FindAnnotation(RelationalAnnotationNames.Filter) is IConventionAnnotation { Value: not null } filter
+            && filter.GetConfigurationSource() != ConfigurationSource.Convention;
+}

src/EFCore/EFCore.baseline.json

@@ -8827,6 +8827,9 @@
         {
           "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.IConventionIndex? CreateIndex(System.Collections.Generic.IReadOnlyList<Microsoft.EntityFrameworkCore.Metadata.IConventionProperty> properties, bool unique, Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionEntityTypeBuilder entityTypeBuilder);"
         },
+        {
+          "Member": "virtual bool IsPartial(Microsoft.EntityFrameworkCore.Metadata.IConventionIndex index);"
+        },
         {
           "Member": "virtual void ProcessEntityTypeBaseTypeChanged(Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionEntityTypeBuilder entityTypeBuilder, Microsoft.EntityFrameworkCore.Metadata.IConventionEntityType? newBaseType, Microsoft.EntityFrameworkCore.Metadata.IConventionEntityType? oldBaseType, Microsoft.EntityFrameworkCore.Metadata.Conventions.IConventionContext<Microsoft.EntityFrameworkCore.Metadata.IConventionEntityType> context);"
         },
@@ -8845,6 +8848,9 @@
         {
           "Member": "virtual void ProcessIndexAdded(Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionIndexBuilder indexBuilder, Microsoft.EntityFrameworkCore.Metadata.Conventions.IConventionContext<Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionIndexBuilder> context);"
         },
+        {
+          "Member": "virtual void ProcessIndexCoverageChanged(Microsoft.EntityFrameworkCore.Metadata.IConventionIndex index);"
+        },
         {
           "Member": "virtual void ProcessIndexRemoved(Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionEntityTypeBuilder entityTypeBuilder, Microsoft.EntityFrameworkCore.Metadata.IConventionIndex index, Microsoft.EntityFrameworkCore.Metadata.Conventions.IConventionContext<Microsoft.EntityFrameworkCore.Metadata.IConventionIndex> context);"
         },

src/EFCore/Metadata/Conventions/ForeignKeyIndexConvention.cs

@@ -195,8 +195,7 @@ public virtual void ProcessEntityTypeBaseTypeChanged(
                 }
                 else
                 {
-                    var coveringIndex = baseIndexes!.FirstOrDefault(i => AreIndexedBy(
-                        foreignKey.Properties, foreignKey.IsUnique, i.Properties, i.IsUnique));
+                    var coveringIndex = baseIndexes!.FirstOrDefault(i => AreIndexedBy(foreignKey.Properties, foreignKey.IsUnique, i));
                     if (coveringIndex != null)
                     {
                         RemoveIndex(index);
@@ -221,6 +220,55 @@ public virtual void ProcessIndexAdded(IConventionIndexBuilder indexBuilder, ICon
             return;
         }
 
+        RemoveCoveredIndexes(index);
+    }
+
+    /// <summary>
+    ///     Called when something other than the properties or uniqueness of an index changes which may affect whether
+    ///     the index covers the rows of the table. For example, a filter being added to or removed from the index.
+    /// </summary>
+    /// <remarks>
+    ///     When the index no longer covers all rows, any foreign key indexes that were removed because they were covered by it
+    ///     are re-created. When the index starts covering all rows, any indexes that it now covers are removed.
+    /// </remarks>
+    /// <param name="index">The index.</param>
+    protected virtual void ProcessIndexCoverageChanged(IConventionIndex index)
+    {
+        if (!index.IsInModel
+            || index.CollectionIndices is not null)
+        {
+            return;
+        }
+
+        if (IsPartial(index))
+        {
+            CreateCoveredForeignKeyIndexes(index);
+        }
+        else
+        {
+            RemoveCoveredIndexes(index);
+        }
+    }
+
+    /// <summary>
+    ///     Returns a value indicating whether the given index only includes a subset of the rows in the table, and so
+    ///     cannot be used in place of an index that includes all rows.
+    /// </summary>
+    /// <remarks>
+    ///     This is always <see langword="false" /> by default. Relational providers override this to account for index filters.
+    /// </remarks>
+    /// <param name="index">The index.</param>
+    /// <returns><see langword="true" /> if the index only includes a subset of the rows.</returns>
+    protected virtual bool IsPartial(IConventionIndex index)
+        => false;
+
+    private void RemoveCoveredIndexes(IConventionIndex index)
+    {
+        if (IsPartial(index))
+        {
+            return;
+        }
+
         foreach (var otherIndex in index.DeclaringEntityType.GetDerivedTypesInclusive()
                      .SelectMany(t => t.GetDeclaredIndexes())
                      .Where(i => i != index
@@ -231,6 +279,24 @@ public virtual void ProcessIndexAdded(IConventionIndexBuilder indexBuilder, ICon
         }
     }
 
+    private void CreateCoveredForeignKeyIndexes(IConventionIndex index)
+    {
+        foreach (var foreignKey in index.DeclaringEntityType.GetDerivedTypesInclusive()
+                     .SelectMany(t => t.GetDeclaredForeignKeys())
+                     .Where(fk => AreIndexedBy(fk.Properties, fk.IsUnique, index.Properties, index.IsUnique)).ToList())
+        {
+            CreateIndex(foreignKey.Properties, foreignKey.IsUnique, foreignKey.DeclaringEntityType.Builder);
+        }
+    }
+
+    private bool AreIndexedBy(
+        IReadOnlyList<IConventionPropertyBase> properties,
+        bool unique,
+        IConventionIndex coveringIndex)
+        => coveringIndex.CollectionIndices is null
+            && !IsPartial(coveringIndex)
+            && AreIndexedBy(properties, unique, coveringIndex.Properties, coveringIndex.IsUnique);
+
     /// <summary>
     ///     Called after an index is removed.
     /// </summary>
@@ -248,17 +314,14 @@ public virtual void ProcessIndexRemoved(
         }
 
         // A removed complex index never covered any FK index, so nothing to re-create.
-        if (index.CollectionIndices is not null)
+        // Likewise, a partial index never caused a FK index to be removed.
+        if (index.CollectionIndices is not null
+            || IsPartial(index))
         {
             return;
         }
 
-        foreach (var foreignKey in index.DeclaringEntityType.GetDerivedTypesInclusive()
-                     .SelectMany(t => t.GetDeclaredForeignKeys())
-                     .Where(fk => AreIndexedBy(fk.Properties, fk.IsUnique, index.Properties, index.IsUnique)))
-        {
-            CreateIndex(foreignKey.Properties, foreignKey.IsUnique, foreignKey.DeclaringEntityType.Builder);
-        }
+        CreateCoveredForeignKeyIndexes(index);
     }
 
     /// <summary>
@@ -292,8 +355,7 @@ public virtual void ProcessForeignKeyUniquenessChanged(
                 }
 
                 var coveringIndex = foreignKey.DeclaringEntityType.GetIndexes()
-                    .FirstOrDefault(i => i.CollectionIndices is null
-                        && AreIndexedBy(foreignKey.Properties, false, i.Properties, i.IsUnique));
+                    .FirstOrDefault(i => AreIndexedBy(foreignKey.Properties, false, i));
                 if (coveringIndex != null)
                 {
                     RemoveIndex(index);
@@ -315,7 +377,8 @@ public virtual void ProcessIndexUniquenessChanged(
         IConventionContext<bool?> context)
     {
         var index = indexBuilder.Metadata;
-        if (index.CollectionIndices is not null)
+        if (index.CollectionIndices is not null
+            || IsPartial(index))
         {
             return;
         }
@@ -366,8 +429,7 @@ public virtual void ProcessIndexUniquenessChanged(
 
         foreach (var existingIndex in entityTypeBuilder.Metadata.GetIndexes())
         {
-            if (existingIndex.CollectionIndices is null
-                && AreIndexedBy(properties, unique, existingIndex.Properties, existingIndex.IsUnique))
+            if (AreIndexedBy(properties, unique, existingIndex))
             {
                 return null;
             }
@@ -431,10 +493,7 @@ public virtual void ProcessModelFinalizing(
 
                 foreach (var existingIndex in entityType.GetIndexes())
                 {
-                    if (existingIndex.CollectionIndices is null
-                        && AreIndexedBy(
-                            declaredForeignKey.Properties, declaredForeignKey.IsUnique, existingIndex.Properties,
-                            existingIndex.IsUnique))
+                    if (AreIndexedBy(declaredForeignKey.Properties, declaredForeignKey.IsUnique, existingIndex))
                     {
                         if (declaredForeignKey.Properties.Count != existingIndex.Properties.Count)
                         {

test/EFCore.Relational.Specification.Tests/Scaffolding/CompiledModelRelationalTestBase.cs

@@ -911,7 +911,10 @@ protected virtual void AssertTpcSprocs(IModel model)
         Assert.Equal("PIX", alternateIndex.GetDatabaseName());
         Assert.Equal("AlternateId <> NULL", alternateIndex.GetFilter());
 
-        Assert.Equal([alternateIndex], principalBaseId.GetContainingIndexes());
+        // The filtered unique index doesn't cover all rows, so the foreign key also has an index of its own.
+        var foreignKeyIndex = principalBase.FindIndex(principalBaseId)!;
+        Assert.NotSame(alternateIndex, foreignKeyIndex);
+        Assert.Equal([foreignKeyIndex, alternateIndex], principalBaseId.GetContainingIndexes());
 
         var insertSproc = principalBase.GetInsertStoredProcedure()!;
         Assert.Equal("PrincipalBase_Insert", insertSproc.Name);