Don't issue a warning when replacing an optional dependent with non-n…

…ull values Fixes #26712
dotnet · Jan 7, 2022 · 95ca59e · 95ca59e
1 parent 7e94966
commit 95ca59e
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 7 deletions.
diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs
@@ -282,7 +282,7 @@ private List<IColumnModification> GenerateColumnModifications()
             }
 
             var optionalDependentWithAllNull =
-                (entry.EntityState == EntityState.Deleted
+                (entry.EntityState == EntityState.Modified
                     || entry.EntityState == EntityState.Added)
                 && tableMapping.Table.IsOptional(entry.EntityType)
                 && tableMapping.Table.GetRowInternalForeignKeys(entry.EntityType).Any();
@@ -352,12 +352,19 @@ private List<IColumnModification> GenerateColumnModifications()
                     columnModifications.Add(columnModification);
 
                     if (optionalDependentWithAllNull
-                        && columnModification.IsWrite
-                        && (entry.EntityState != EntityState.Added || columnModification.Value is not null))
+                        && (columnModification.IsWrite
+                            || (columnModification.IsCondition && !isKey))
+                        && columnModification.Value is not null)
                     {
                         optionalDependentWithAllNull = false;
                     }
                 }
+                else if (optionalDependentWithAllNull
+                    && state == EntityState.Modified
+                    && entry.GetCurrentValue(property) is not null)
+                {
+                    optionalDependentWithAllNull = false;
+                }
             }
 
             if (optionalDependentWithAllNull && _logger != null)

diff --git a/test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs b/test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs
@@ -578,6 +578,22 @@ public virtual async Task Warn_when_save_optional_dependent_with_null_values()
         var log = TestSqlLoggerFactory.Log.Single(l => l.Level == LogLevel.Warning);
 
         Assert.Equal(expected, log.Message);
+
+        TestSqlLoggerFactory.Clear();
+
+        meterReading.MeterReadingDetails = new MeterReadingDetail { CurrentRead = "100" };
+
+        context.SaveChanges();
+
+        Assert.Empty(TestSqlLoggerFactory.Log.Where(l => l.Level == LogLevel.Warning));
+
+        meterReading.MeterReadingDetails = new MeterReadingDetail();
+
+        context.SaveChanges();
+
+        log = TestSqlLoggerFactory.Log.Single(l => l.Level == LogLevel.Warning);
+
+        Assert.Equal(expected, log.Message);
     }
 
     [ConditionalFact]
@@ -595,9 +611,11 @@ public virtual async Task No_warn_when_save_optional_dependent_at_least_one_none
 
         context.SaveChanges();
 
-        var log = TestSqlLoggerFactory.Log.SingleOrDefault(l => l.Level == LogLevel.Warning);
+        meterReading.MeterReadingDetails = new MeterReadingDetail { CurrentRead = "100" };
+
+        context.SaveChanges();
 
-        Assert.Null(log.Message);
+        Assert.Empty(TestSqlLoggerFactory.Log.Where(l => l.Level == LogLevel.Warning));
     }
 
     protected override string StoreName { get; } = "TableSplittingTest";
@@ -684,8 +702,8 @@ public SharedTableContext(DbContextOptions options)
         {
         }
 
-        protected DbSet<MeterReading> MeterReadings { get; set; }
-        protected DbSet<MeterReadingDetail> MeterReadingDetails { get; set; }
+        public DbSet<MeterReading> MeterReadings { get; set; }
+        public DbSet<MeterReadingDetail> MeterReadingDetails { get; set; }
     }
 
     protected class MeterReading