Make unique index SaveChanges topological dependency optional for nul…

…l values

Fixes #29647

src/EFCore.Relational/Update/Internal/CompositeRowIndexValueFactory.cs

@@ -35,17 +35,23 @@ public CompositeRowIndexValueFactory(ITableIndex index)
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual bool TryCreateIndexValue(object?[] keyValues, [NotNullWhen(true)] out object?[]? key)
-        => TryCreateDependentKeyValue(keyValues, out key);
+    public virtual bool TryCreateIndexValue(
+        object?[] keyValues,
+        out object?[]? key,
+        out bool hasNullValue)
+        => TryCreateDependentKeyValue(keyValues, out key, out hasNullValue);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual bool TryCreateIndexValue(IDictionary<string, object?> keyValues, [NotNullWhen(true)] out object?[]? key)
-        => TryCreateDependentKeyValue(keyValues, out key);
+    public virtual bool TryCreateIndexValue(
+        IDictionary<string, object?> keyValues,
+        out object?[]? key,
+        out bool hasNullValue)
+        => TryCreateDependentKeyValue(keyValues, out key, out hasNullValue);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -56,28 +62,31 @@ public virtual bool TryCreateIndexValue(IDictionary<string, object?> keyValues,
     public virtual bool TryCreateIndexValue(
         IReadOnlyModificationCommand command,
         bool fromOriginalValues,
-        [NotNullWhen(true)] out object?[]? key)
-        => TryCreateDependentKeyValue(command, fromOriginalValues, out key);
+        out object?[]? keyValue,
+        out bool hasNullValue)
+        => TryCreateDependentKeyValue(command, fromOriginalValues, out keyValue, out hasNullValue);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual object? CreateEquatableIndexValue(IReadOnlyModificationCommand command, bool fromOriginalValues = false)
-        => TryCreateDependentKeyValue(command, fromOriginalValues, out var keyValue)
-            ? new EquatableKeyValue<object?[]>(_index, keyValue, EqualityComparer)
-            : null;
+    public virtual (object? Value, bool HasNullValue) CreateEquatableIndexValue(
+        IReadOnlyModificationCommand command, bool fromOriginalValues = false)
+        => TryCreateIndexValue(command, fromOriginalValues, out var keyValue, out var hasNullValue)
+            ? (new EquatableKeyValue<object?[]>(_index, keyValue, EqualityComparer), hasNullValue)
+            : (null, true);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual object[]? CreateIndexValue(IReadOnlyModificationCommand command, bool fromOriginalValues = false)
-        => TryCreateIndexValue(command, fromOriginalValues, out var keyValue)
-            ? (object[])keyValue
-            : null;
+    public virtual (object?[]? Value, bool HasNullValue) CreateIndexValue(
+        IReadOnlyModificationCommand command, bool fromOriginalValues = false)
+        => TryCreateIndexValue(command, fromOriginalValues, out var keyValue, out bool hasNullValue)
+            ? (keyValue, hasNullValue)
+            : (null, true);
 }

src/EFCore.Relational/Update/Internal/CompositeRowKeyValueFactory.cs

@@ -73,15 +73,15 @@ public CompositeRowKeyValueFactory(IUniqueConstraint key)
     /// </summary>
     public virtual object?[] CreateKeyValue(IReadOnlyModificationCommand command, bool fromOriginalValues = false)
     {
-        if (!TryCreateDependentKeyValue(command, fromOriginalValues, out var key))
+        if (!TryCreateDependentKeyValue(command, fromOriginalValues, out var keyValue))
         {
             throw new InvalidOperationException(
                 RelationalStrings.NullKeyValue(
                     _constraint.Table.SchemaQualifiedName,
-                    FindNullColumnInKeyValues(key).Name));
+                    FindNullColumnInKeyValues(keyValue).Name));
         }
 
-        return key;
+        return keyValue;
     }
 
     private IColumn FindNullColumnInKeyValues(object?[]? keyValues)

src/EFCore.Relational/Update/Internal/CompositeRowValueFactory.cs

@@ -55,9 +55,23 @@ protected CompositeRowValueFactory(IReadOnlyList<IColumn> columns)
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual bool TryCreateDependentKeyValue(object?[] keyValues, [NotNullWhen(true)] out object?[]? key)
+        => TryCreateDependentKeyValue(keyValues, out key, out var hasNullValue)
+            && !hasNullValue;
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected virtual bool TryCreateDependentKeyValue(
+        object?[] keyValues,
+        [NotNullWhen(true)] out object?[]? key,
+        out bool hasNullValue)
     {
         key = keyValues;
-        return keyValues.All(k => k != null);
+        hasNullValue = keyValues.All(k => k != null);
+        return true;
     }
 
     /// <summary>
@@ -67,18 +81,36 @@ public virtual bool TryCreateDependentKeyValue(object?[] keyValues, [NotNullWhen
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual bool TryCreateDependentKeyValue(IDictionary<string, object?> keyValues, [NotNullWhen(true)] out object?[]? key)
+        => TryCreateDependentKeyValue(keyValues, out key, out var hasNullValue)
+            && !hasNullValue;
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected virtual bool TryCreateDependentKeyValue(
+        IDictionary<string, object?> keyValues,
+        [NotNullWhen(true)] out object?[]? key,
+        out bool hasNullValue)
     {
         key = new object[Columns.Count];
         var index = 0;
+        hasNullValue = false;
 
         foreach (var column in Columns)
         {
-            if (!keyValues.TryGetValue(column.Name, out var value)
-                || value == null)
+            if (!keyValues.TryGetValue(column.Name, out var value))
             {
                 return false;
             }
 
+            if (value == null)
+            {
+                hasNullValue = true;
+            }
+
             key[index++] = value;
         }
 
@@ -95,10 +127,34 @@ public virtual bool TryCreateDependentKeyValue(
         IReadOnlyModificationCommand command,
         bool fromOriginalValues,
         [NotNullWhen(true)] out object?[]? key)
+    {
+        var result = TryCreateDependentKeyValue(command, fromOriginalValues, out key, out var hasNullValue);
+        if (!result
+            || hasNullValue)
+        {
+            key = null;
+            return result;
+        }
+
+        return result;
+    }
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected virtual bool TryCreateDependentKeyValue(
+        IReadOnlyModificationCommand command,
+        bool fromOriginalValues,
+        [NotNullWhen(true)] out object?[]? key,
+        out bool hasNullValue)
     {
         var converters = ValueConverters;
         key = new object[Columns.Count];
         var index = 0;
+        hasNullValue = false;
 
         for (var i = 0; i < Columns.Count; i++)
         {
@@ -144,6 +200,11 @@ public virtual bool TryCreateDependentKeyValue(
                     return false;
                 }
 
+                if (value == null)
+                {
+                    hasNullValue = true;
+                }
+
                 key[index++] = value;
             }
             else
@@ -155,6 +216,11 @@ public virtual bool TryCreateDependentKeyValue(
                 }
 
                 var value = fromOriginalValues ? modification.OriginalValue : modification.Value;
+                if (value == null)
+                {
+                    hasNullValue = true;
+                }
+
                 key[index++] = value;
             }
         }

src/EFCore.Relational/Update/Internal/IRowIndexValueFactory.cs

@@ -17,13 +17,13 @@ public interface IRowIndexValueFactory
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    object[]? CreateIndexValue(IReadOnlyModificationCommand command, bool fromOriginalValues = false);
+    (object?[]? Value, bool HasNullValue) CreateIndexValue(IReadOnlyModificationCommand command, bool fromOriginalValues = false);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    object? CreateEquatableIndexValue(IReadOnlyModificationCommand command, bool fromOriginalValues = false);
+    (object? Value, bool HasNullValue) CreateEquatableIndexValue(IReadOnlyModificationCommand command, bool fromOriginalValues = false);
 }

src/EFCore.Relational/Update/Internal/IRowIndexValueFactory`.cs

@@ -19,23 +19,23 @@ public interface IRowIndexValueFactory<TKey> : IRowIndexValueFactory
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    bool TryCreateIndexValue(object?[] keyValues, [NotNullWhen(true)] out TKey? key);
+    bool TryCreateIndexValue(object?[] keyValues, out TKey? key, out bool hasNullValue);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    bool TryCreateIndexValue(IDictionary<string, object?> keyValues, [NotNullWhen(true)] out TKey? key);
+    bool TryCreateIndexValue(IDictionary<string, object?> keyValues, out TKey? key, out bool hasNullValue);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    bool TryCreateIndexValue(IReadOnlyModificationCommand command, bool fromOriginalValues, [NotNullWhen(true)] out TKey? key);
+    bool TryCreateIndexValue(IReadOnlyModificationCommand command, bool fromOriginalValues, out TKey? key, out bool hasNullValue);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to

src/EFCore.Relational/Update/Internal/SimpleRowIndexValueFactory.cs

@@ -49,10 +49,11 @@ public SimpleRowIndexValueFactory(ITableIndex index)
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual bool TryCreateIndexValue(object?[] keyValues, [NotNullWhen(true)] out TKey? key)
+    public virtual bool TryCreateIndexValue(object?[] keyValues, out TKey? key, out bool hasNullValue)
     {
         key = (TKey?)keyValues[0];
-        return key != null;
+        hasNullValue = key == null;
+        return true;
     }
 
     /// <summary>
@@ -61,15 +62,17 @@ public virtual bool TryCreateIndexValue(object?[] keyValues, [NotNullWhen(true)]
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual bool TryCreateIndexValue(IDictionary<string, object?> keyValues, [NotNullWhen(true)] out TKey? key)
+    public virtual bool TryCreateIndexValue(IDictionary<string, object?> keyValues, out TKey? key, out bool hasNullValue)
     {
         if (keyValues.TryGetValue(_column.Name, out var value))
         {
             key = (TKey?)value;
-            return key != null;
+            hasNullValue = key == null;
+            return true;
         }
 
         key = default;
+        hasNullValue = true;
         return false;
     }
 
@@ -82,12 +85,14 @@ public virtual bool TryCreateIndexValue(IDictionary<string, object?> keyValues,
     public virtual bool TryCreateIndexValue(
         IReadOnlyModificationCommand command,
         bool fromOriginalValues,
-        [NotNullWhen(true)] out TKey? key)
+        out TKey? key,
+        out bool hasNullValue)
     {
         (key, var present) = fromOriginalValues
             ? ((Func<IReadOnlyModificationCommand, (TKey, bool)>)_columnAccessors.OriginalValueGetter)(command)
             : ((Func<IReadOnlyModificationCommand, (TKey, bool)>)_columnAccessors.CurrentValueGetter)(command);
-        return present && key != null;
+        hasNullValue = key == null;
+        return present;
     }
 
     /// <summary>
@@ -96,19 +101,21 @@ public virtual bool TryCreateIndexValue(
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual object? CreateEquatableIndexValue(IReadOnlyModificationCommand command, bool fromOriginalValues = false)
-        => TryCreateIndexValue(command, fromOriginalValues, out var keyValue)
-            ? new EquatableKeyValue<TKey>(_index, keyValue, EqualityComparer)
-            : null;
+    public virtual (object? Value, bool HasNullValue) CreateEquatableIndexValue(
+        IReadOnlyModificationCommand command, bool fromOriginalValues = false)
+        => TryCreateIndexValue(command, fromOriginalValues, out var keyValue, out var hasNullValue)
+            ? (new EquatableKeyValue<TKey>(_index, keyValue, EqualityComparer), hasNullValue)
+            : (null, true);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual object[]? CreateIndexValue(IReadOnlyModificationCommand command, bool fromOriginalValues = false)
-        => TryCreateIndexValue(command, fromOriginalValues, out var value)
-            ? (new object[] { value })
-            : null;
+    public virtual (object?[]? Value, bool HasNullValue) CreateIndexValue(
+        IReadOnlyModificationCommand command, bool fromOriginalValues = false)
+        => TryCreateIndexValue(command, fromOriginalValues, out var value, out var hasNullValue)
+            ? (new object?[] { value }, hasNullValue)
+            : (null, true);
 }

src/EFCore/Update/EquatableKeyValue.cs

@@ -11,7 +11,7 @@ namespace Microsoft.EntityFrameworkCore.Update;
 public sealed class EquatableKeyValue<TKey>
 {
     private readonly IAnnotatable _metadata;
-    private readonly TKey _keyValue;
+    private readonly TKey? _keyValue;
     private readonly IEqualityComparer<TKey> _keyComparer;
 
     /// <summary>
@@ -22,7 +22,7 @@ public sealed class EquatableKeyValue<TKey>
     /// <param name="keyComparer">The key comparer.</param>
     public EquatableKeyValue(
         IAnnotatable metadata,
-        TKey keyValue,
+        TKey? keyValue,
         IEqualityComparer<TKey> keyComparer)
     {
         _metadata = metadata;
@@ -44,7 +44,11 @@ public override int GetHashCode()
     {
         var hash = new HashCode();
         hash.Add(_metadata);
-        hash.Add(_keyValue, _keyComparer);
+        if (_keyValue != null)
+        {
+            hash.Add(_keyValue, _keyComparer);
+        }
+
         return hash.ToHashCode();
     }
 }

test/EFCore.Relational.Specification.Tests/UpdatesRelationalTestBase.cs

@@ -172,7 +172,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
             modelBuilder.Entity<ProductTableWithView>().HasBaseType((string)null).ToView("ProductView").ToTable("ProductTable");
             modelBuilder.Entity<ProductTableView>().HasBaseType((string)null).ToView("ProductTable");
 
-            modelBuilder.Entity<Product>().HasIndex(p => new { p.Name, p.Price }).IsUnique().HasFilter("Name IS NOT NULL");
+            modelBuilder.Entity<Product>().HasIndex(p => new { p.Name, p.Price }).IsUnique();
 
             modelBuilder.Entity<Person>()
                 .Property(p => p.Country)

test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTestBase.cs

@@ -245,6 +245,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
 
             modelBuilder.Entity<ProductBase>()
                 .Property(p => p.Id).HasDefaultValueSql("NEWID()");
+
+            modelBuilder.Entity<Product>().HasIndex(p => new { p.Name, p.Price }).HasFilter("Name IS NOT NULL");
         }
         public virtual void ResetIdentity()
         {