From 69f4f2a9248124cf7c22c215067b36a564f4d89b Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Fri, 31 Jul 2026 17:28:26 +0500 Subject: [PATCH 01/11] Add IsOptional() metadata for entity-splitting fragments Part of #27974 --- .../OwnedNavigationSplitTableBuilder.cs | 23 ++++++ .../OwnedNavigationSplitTableBuilder``.cs | 19 +++++ .../Metadata/Builders/SplitTableBuilder.cs | 23 ++++++ .../Metadata/Builders/SplitTableBuilder`.cs | 19 +++++ .../IConventionEntityTypeMappingFragment.cs | 14 ++++ .../IMutableEntityTypeMappingFragment.cs | 7 ++ .../IReadOnlyEntityTypeMappingFragment.cs | 13 ++++ .../Internal/EntityTypeMappingFragment.cs | 56 ++++++++++++++ ...nternalEntityTypeMappingFragmentBuilder.cs | 31 ++++++++ .../Metadata/RelationalAnnotationNames.cs | 6 ++ .../RuntimeEntityTypeMappingFragment.cs | 15 +++- .../RelationalModelBuilderTest.cs | 74 +++++++++++++++++++ 12 files changed, 299 insertions(+), 1 deletion(-) diff --git a/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder.cs b/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder.cs index 419ffe2d0c7..30704c50630 100644 --- a/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder.cs +++ b/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder.cs @@ -80,6 +80,29 @@ public virtual OwnedNavigationSplitTableBuilder ExcludeFromMigrations(bool exclu return this; } + /// + /// Configures whether a row for this table might not exist even when the principal row exists in the main table + /// for the entity type. + /// + /// + /// + /// When a fragment is optional, a row is only created for it when at least one non-key property mapped to the + /// fragment has a non- value. All non-key properties mapped to an optional fragment + /// must be configured as nullable for that table. + /// + /// + /// See Table splitting for more information and examples. + /// + /// + /// A value indicating whether a row for this table is optional. + /// The same builder instance so that multiple calls can be chained. + public virtual OwnedNavigationSplitTableBuilder IsOptional(bool optional = true) + { + MappingFragment.IsOptional = optional; + + return this; + } + /// /// Configures a database trigger on the table. /// diff --git a/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder``.cs b/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder``.cs index 9ec9782da31..10689e2f556 100644 --- a/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder``.cs +++ b/src/EFCore.Relational/Metadata/Builders/OwnedNavigationSplitTableBuilder``.cs @@ -44,6 +44,25 @@ private OwnedNavigationBuilder OwnedNavigationBu public new virtual OwnedNavigationSplitTableBuilder ExcludeFromMigrations(bool excluded = true) => (OwnedNavigationSplitTableBuilder)base.ExcludeFromMigrations(excluded); + /// + /// Configures whether a row for this table might not exist even when the principal row exists in the main table + /// for the entity type. + /// + /// + /// + /// When a fragment is optional, a row is only created for it when at least one non-key property mapped to the + /// fragment has a non- value. All non-key properties mapped to an optional fragment + /// must be configured as nullable for that table. + /// + /// + /// See Table splitting for more information and examples. + /// + /// + /// A value indicating whether a row for this table is optional. + /// The same builder instance so that multiple calls can be chained. + public new virtual OwnedNavigationSplitTableBuilder IsOptional(bool optional = true) + => (OwnedNavigationSplitTableBuilder)base.IsOptional(optional); + /// /// Maps the property to a column on the current table and returns an object that can be used /// to provide table-specific configuration if the property is mapped to more than one table. diff --git a/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder.cs b/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder.cs index 03736db6abc..0a9a900f929 100644 --- a/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder.cs +++ b/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder.cs @@ -80,6 +80,29 @@ public virtual SplitTableBuilder ExcludeFromMigrations(bool excluded = true) return this; } + /// + /// Configures whether a row for this table might not exist even when the principal row exists in the main table + /// for the entity type. + /// + /// + /// + /// When a fragment is optional, a row is only created for it when at least one non-key property mapped to the + /// fragment has a non- value. All non-key properties mapped to an optional fragment + /// must be configured as nullable for that table. + /// + /// + /// See Table splitting for more information and examples. + /// + /// + /// A value indicating whether a row for this table is optional. + /// The same builder instance so that multiple calls can be chained. + public virtual SplitTableBuilder IsOptional(bool optional = true) + { + MappingFragment.IsOptional = optional; + + return this; + } + /// /// Configures a database trigger on the table. /// diff --git a/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder`.cs b/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder`.cs index c1b1220724f..b3a773aad15 100644 --- a/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder`.cs +++ b/src/EFCore.Relational/Metadata/Builders/SplitTableBuilder`.cs @@ -37,6 +37,25 @@ private EntityTypeBuilder EntityTypeBuilder public new virtual SplitTableBuilder ExcludeFromMigrations(bool excluded = true) => (SplitTableBuilder)base.ExcludeFromMigrations(excluded); + /// + /// Configures whether a row for this table might not exist even when the principal row exists in the main table + /// for the entity type. + /// + /// + /// + /// When a fragment is optional, a row is only created for it when at least one non-key property mapped to the + /// fragment has a non- value. All non-key properties mapped to an optional fragment + /// must be configured as nullable for that table. + /// + /// + /// See Table splitting for more information and examples. + /// + /// + /// A value indicating whether a row for this table is optional. + /// The same builder instance so that multiple calls can be chained. + public new virtual SplitTableBuilder IsOptional(bool optional = true) + => (SplitTableBuilder)base.IsOptional(optional); + /// /// Maps the property to a column on the current table and returns an object that can be used /// to provide table-specific configuration if the property is mapped to more than one table. diff --git a/src/EFCore.Relational/Metadata/IConventionEntityTypeMappingFragment.cs b/src/EFCore.Relational/Metadata/IConventionEntityTypeMappingFragment.cs index ff3f453c233..af7403da0a3 100644 --- a/src/EFCore.Relational/Metadata/IConventionEntityTypeMappingFragment.cs +++ b/src/EFCore.Relational/Metadata/IConventionEntityTypeMappingFragment.cs @@ -40,4 +40,18 @@ public interface IConventionEntityTypeMappingFragment : IReadOnlyEntityTypeMappi /// /// The for . ConfigurationSource? GetIsTableExcludedFromMigrationsConfigurationSource(); + + /// + /// Sets a value indicating whether a row might not exist for this fragment's store object even when the + /// principal row exists in the main table for the entity type. + /// + /// A value indicating whether a row for this fragment is optional. + /// Indicates whether the configuration was specified using a data annotation. + bool? SetIsOptional(bool? optional, bool fromDataAnnotation = false); + + /// + /// Gets the for . + /// + /// The for . + ConfigurationSource? GetIsOptionalConfigurationSource(); } diff --git a/src/EFCore.Relational/Metadata/IMutableEntityTypeMappingFragment.cs b/src/EFCore.Relational/Metadata/IMutableEntityTypeMappingFragment.cs index 9a88f1a8365..83d3007de47 100644 --- a/src/EFCore.Relational/Metadata/IMutableEntityTypeMappingFragment.cs +++ b/src/EFCore.Relational/Metadata/IMutableEntityTypeMappingFragment.cs @@ -21,4 +21,11 @@ public interface IMutableEntityTypeMappingFragment : IReadOnlyEntityTypeMappingF /// /// A value indicating whether the associated table is ignored by Migrations. new bool? IsTableExcludedFromMigrations { get; set; } + + /// + /// Gets or sets a value indicating whether a row might not exist for this fragment's store object even when the + /// principal row exists in the main table for the entity type. + /// + /// if a row for this fragment is optional; otherwise. + new bool IsOptional { get; set; } } diff --git a/src/EFCore.Relational/Metadata/IReadOnlyEntityTypeMappingFragment.cs b/src/EFCore.Relational/Metadata/IReadOnlyEntityTypeMappingFragment.cs index 591f6e55291..d03835e54b1 100644 --- a/src/EFCore.Relational/Metadata/IReadOnlyEntityTypeMappingFragment.cs +++ b/src/EFCore.Relational/Metadata/IReadOnlyEntityTypeMappingFragment.cs @@ -29,6 +29,14 @@ public interface IReadOnlyEntityTypeMappingFragment : IReadOnlyAnnotatable /// A value indicating whether the associated table is ignored by Migrations. bool? IsTableExcludedFromMigrations { get; } + /// + /// Gets a value indicating whether a row might not exist for this fragment's store object even when the + /// principal row exists in the main table for the entity type. + /// + /// if a row for this fragment is optional; otherwise. + bool IsOptional + => false; + /// /// /// Creates a human-readable representation of the given metadata. @@ -56,6 +64,11 @@ string ToDebugString(MetadataDebugStringOptions options = MetadataDebugStringOpt builder.Append("ExcludedFromMigrations"); } + if (IsOptional) + { + builder.Append(" Optional"); + } + if ((options & MetadataDebugStringOptions.SingleLine) == 0) { if ((options & MetadataDebugStringOptions.IncludeAnnotations) != 0) diff --git a/src/EFCore.Relational/Metadata/Internal/EntityTypeMappingFragment.cs b/src/EFCore.Relational/Metadata/Internal/EntityTypeMappingFragment.cs index eed19d44965..e9e3a59c22c 100644 --- a/src/EFCore.Relational/Metadata/Internal/EntityTypeMappingFragment.cs +++ b/src/EFCore.Relational/Metadata/Internal/EntityTypeMappingFragment.cs @@ -16,10 +16,12 @@ public class EntityTypeMappingFragment : IConventionEntityTypeMappingFragment { private bool? _isTableExcludedFromMigrations; + private bool? _isOptional; private InternalEntityTypeMappingFragmentBuilder? _builder; private ConfigurationSource _configurationSource; private ConfigurationSource? _isTableExcludedFromMigrationsConfigurationSource; + private ConfigurationSource? _isOptionalConfigurationSource; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -149,6 +151,14 @@ public static EntityTypeMappingFragment MergeInto( !.Metadata; } + var isOptionalConfigurationSource = detachedFragment.GetIsOptionalConfigurationSource(); + if (isOptionalConfigurationSource != null) + { + existingFragment = ((InternalEntityTypeMappingFragmentBuilder)existingFragment.Builder).SetIsOptional( + detachedFragment.IsOptional, isOptionalConfigurationSource.Value) + !.Metadata; + } + return ((InternalEntityTypeMappingFragmentBuilder)existingFragment.Builder) .MergeAnnotationsFrom((EntityTypeMappingFragment)detachedFragment) .Metadata; @@ -191,6 +201,43 @@ public virtual bool? IsTableExcludedFromMigrations public virtual ConfigurationSource? GetIsTableExcludedFromMigrationsConfigurationSource() => _isTableExcludedFromMigrationsConfigurationSource; + /// + public virtual bool IsOptional + { + get => _isOptional ?? false; + set => SetIsOptional(value, ConfigurationSource.Explicit); + } + + /// + /// 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. + /// + public virtual bool? SetIsOptional(bool? optional, ConfigurationSource configurationSource) + { + if (!configurationSource.Overrides(_isOptionalConfigurationSource)) + { + return null; + } + + _isOptional = optional; + _isOptionalConfigurationSource = + optional == null + ? null + : configurationSource.Max(_isOptionalConfigurationSource); + return optional; + } + + /// + /// 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. + /// + public virtual ConfigurationSource? GetIsOptionalConfigurationSource() + => _isOptionalConfigurationSource; + /// /// 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 @@ -321,12 +368,21 @@ IConventionEntityType IConventionEntityTypeMappingFragment.EntityType => SetIsTableExcludedFromMigrations( excluded, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + bool? IConventionEntityTypeMappingFragment.SetIsOptional(bool? optional, bool fromDataAnnotation) + => SetIsOptional(optional, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + bool? IReadOnlyEntityTypeMappingFragment.IsTableExcludedFromMigrations { [DebuggerStepThrough] get => IsTableExcludedFromMigrations; } + bool IReadOnlyEntityTypeMappingFragment.IsOptional + { + [DebuggerStepThrough] + get => IsOptional; + } + IConventionEntityTypeMappingFragmentBuilder IConventionEntityTypeMappingFragment.Builder { [DebuggerStepThrough] diff --git a/src/EFCore.Relational/Metadata/Internal/InternalEntityTypeMappingFragmentBuilder.cs b/src/EFCore.Relational/Metadata/Internal/InternalEntityTypeMappingFragmentBuilder.cs index 7f60361e2c0..545f2b8fc97 100644 --- a/src/EFCore.Relational/Metadata/Internal/InternalEntityTypeMappingFragmentBuilder.cs +++ b/src/EFCore.Relational/Metadata/Internal/InternalEntityTypeMappingFragmentBuilder.cs @@ -57,6 +57,37 @@ public virtual bool CanExcludeTableFromMigrations( => configurationSource.Overrides(Metadata.GetIsTableExcludedFromMigrationsConfigurationSource()) || Metadata.IsTableExcludedFromMigrations == excludedFromMigrations; + /// + /// 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. + /// + public virtual InternalEntityTypeMappingFragmentBuilder? SetIsOptional( + bool? optional, + ConfigurationSource configurationSource) + { + if (!CanSetIsOptional(optional, configurationSource)) + { + return null; + } + + Metadata.SetIsOptional(optional, configurationSource); + return this; + } + + /// + /// 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. + /// + public virtual bool CanSetIsOptional( + bool? optional, + ConfigurationSource configurationSource) + => configurationSource.Overrides(Metadata.GetIsOptionalConfigurationSource()) + || Metadata.IsOptional == optional; + /// IConventionEntityTypeMappingFragment IConventionEntityTypeMappingFragmentBuilder.Metadata { diff --git a/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs b/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs index 11c4f990bcf..181aa9499eb 100644 --- a/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs +++ b/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs @@ -319,6 +319,11 @@ public static class RelationalAnnotationNames /// public const string MappingFragments = Prefix + "MappingFragments"; + /// + /// The name for the annotation that indicates whether an entity type mapping fragment is optional. + /// + public const string EntityTypeMappingFragmentIsOptional = Prefix + "EntityTypeMappingFragmentIsOptional"; + /// /// The name for the annotation that contains table-specific facet overrides. /// @@ -440,6 +445,7 @@ public static class RelationalAnnotationNames TableIndexMappings, UniqueConstraintMappings, MappingFragments, + EntityTypeMappingFragmentIsOptional, RelationalOverrides, ModelDependencies, FieldValueGetter, diff --git a/src/EFCore.Relational/Metadata/RuntimeEntityTypeMappingFragment.cs b/src/EFCore.Relational/Metadata/RuntimeEntityTypeMappingFragment.cs index 2ecbc9b896a..12d40322918 100644 --- a/src/EFCore.Relational/Metadata/RuntimeEntityTypeMappingFragment.cs +++ b/src/EFCore.Relational/Metadata/RuntimeEntityTypeMappingFragment.cs @@ -19,10 +19,14 @@ public class RuntimeEntityTypeMappingFragment : AnnotatableBase, IEntityTypeMapp /// /// A value indicating whether the associated table is ignored by Migrations. /// + /// + /// A value indicating whether a row for this fragment is optional. + /// public RuntimeEntityTypeMappingFragment( RuntimeEntityType entityType, in StoreObjectIdentifier storeObject, - bool? isTableExcludedFromMigrations) + bool? isTableExcludedFromMigrations, + bool isOptional = false) { EntityType = entityType; StoreObject = storeObject; @@ -30,6 +34,11 @@ public RuntimeEntityTypeMappingFragment( { SetAnnotation(RelationalAnnotationNames.IsTableExcludedFromMigrations, isTableExcludedFromMigrations.Value); } + + if (isOptional) + { + SetAnnotation(RelationalAnnotationNames.EntityTypeMappingFragmentIsOptional, true); + } } /// @@ -44,6 +53,10 @@ public RuntimeEntityTypeMappingFragment( public virtual bool? IsTableExcludedFromMigrations => (bool?)this[RelationalAnnotationNames.IsTableExcludedFromMigrations]; + /// + public virtual bool IsOptional + => (bool?)this[RelationalAnnotationNames.EntityTypeMappingFragmentIsOptional] ?? false; + /// public override string ToString() => ((IEntityTypeMappingFragment)this).ToDebugString(MetadataDebugStringOptions.SingleLineDefault); diff --git a/test/EFCore.Relational.Specification.Tests/ModelBuilding/RelationalModelBuilderTest.cs b/test/EFCore.Relational.Specification.Tests/ModelBuilding/RelationalModelBuilderTest.cs index 23455d4a97a..14da6f2a152 100644 --- a/test/EFCore.Relational.Specification.Tests/ModelBuilding/RelationalModelBuilderTest.cs +++ b/test/EFCore.Relational.Specification.Tests/ModelBuilding/RelationalModelBuilderTest.cs @@ -90,6 +90,72 @@ public virtual void Can_use_table_splitting_with_schema() Assert.Null(customerId.GetColumnName(StoreObjectIdentifier.Table("Order"))); } + [Fact] + public virtual void Can_use_optional_table_splitting() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.HasDefaultSchema("dbo"); + + modelBuilder.Entity().SplitToTable( + "OrderDetails", s => + { + s.IsOptional(); + s.Property(o => o.CustomerId); + }); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + + var model = modelBuilder.FinalizeModel(); + + var entity = model.FindEntityType(typeof(Order))!; + var fragment = entity.FindMappingFragment(StoreObjectIdentifier.Table("OrderDetails", "dbo"))!; + + Assert.True(fragment.IsOptional); + } + + [Fact] + public virtual void Can_revert_optional_table_splitting() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.HasDefaultSchema("dbo"); + + modelBuilder.Entity().SplitToTable( + "OrderDetails", s => + { + s.IsOptional(); + s.IsOptional(false); + s.Property(o => o.CustomerId); + }); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + + var model = modelBuilder.FinalizeModel(); + + var entity = model.FindEntityType(typeof(Order))!; + var fragment = entity.FindMappingFragment(StoreObjectIdentifier.Table("OrderDetails", "dbo"))!; + + Assert.False(fragment.IsOptional); + } + + [Fact] + public virtual void Table_splitting_fragment_is_not_optional_by_default() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.HasDefaultSchema("dbo"); + + modelBuilder.Entity().SplitToTable( + "OrderDetails", s => s.Property(o => o.CustomerId)); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + + var model = modelBuilder.FinalizeModel(); + + var entity = model.FindEntityType(typeof(Order))!; + var fragment = entity.FindMappingFragment(StoreObjectIdentifier.Table("OrderDetails", "dbo"))!; + + Assert.False(fragment.IsOptional); + } + [Fact] public virtual void Can_use_view_splitting() { @@ -1854,6 +1920,8 @@ public abstract class TestSplitTableBuilder public abstract TestSplitTableBuilder ExcludeFromMigrations(bool excluded = true); + public abstract TestSplitTableBuilder IsOptional(bool optional = true); + public abstract TestTriggerBuilder HasTrigger(string name); public abstract TestColumnBuilder Property(string propertyName); @@ -1886,6 +1954,9 @@ protected virtual TestSplitTableBuilder Wrap(SplitTableBuilder public override TestSplitTableBuilder ExcludeFromMigrations(bool excluded = true) => Wrap(TableBuilder.ExcludeFromMigrations(excluded)); + public override TestSplitTableBuilder IsOptional(bool optional = true) + => Wrap(TableBuilder.IsOptional(optional)); + public override TestTriggerBuilder HasTrigger(string name) => new NonGenericTestTriggerBuilder(TableBuilder.HasTrigger(name)); @@ -1920,6 +1991,9 @@ protected virtual TestSplitTableBuilder Wrap(SplitTableBuilder tableBui public override TestSplitTableBuilder ExcludeFromMigrations(bool excluded = true) => Wrap(TableBuilder.ExcludeFromMigrations(excluded)); + public override TestSplitTableBuilder IsOptional(bool optional = true) + => Wrap(TableBuilder.IsOptional(optional)); + public override TestTriggerBuilder HasTrigger(string name) => new NonGenericTestTriggerBuilder(TableBuilder.HasTrigger(name)); From b5d272fa4cd1c7c91f71fcd4b253918c951708e3 Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Fri, 31 Jul 2026 17:29:48 +0500 Subject: [PATCH 02/11] Validate optional entity-splitting fragments Part of #27974 --- .../RelationalModelValidator.cs | 21 +++- .../Properties/RelationalStrings.Designer.cs | 16 +++ .../Properties/RelationalStrings.resx | 6 ++ .../RelationalModelValidatorTest.cs | 100 ++++++++++++++++++ 4 files changed, 142 insertions(+), 1 deletion(-) diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index 8229134d304..c1fc08278ea 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -2401,7 +2401,8 @@ protected virtual void ValidateMappingFragment( entityType.DisplayName(), fragment.StoreObject.DisplayName())); } - foreach (var foreignKey in entityType.FindRowInternalForeignKeys(fragment.StoreObject)) + var rowInternalForeignKeys = entityType.FindRowInternalForeignKeys(fragment.StoreObject).ToList(); + foreach (var foreignKey in rowInternalForeignKeys) { var principalMainFragment = StoreObjectIdentifier.Create( foreignKey.PrincipalEntityType, fragment.StoreObject.StoreObjectType)!.Value; @@ -2416,6 +2417,16 @@ protected virtual void ValidateMappingFragment( } } + if (fragment.IsOptional + && rowInternalForeignKeys.Count > 0) + { + throw new InvalidOperationException( + RelationalStrings.EntitySplittingOptionalFragmentSharedTable( + entityType.DisplayName(), + fragment.StoreObject.DisplayName(), + rowInternalForeignKeys[0].PrincipalEntityType.DisplayName())); + } + var propertiesFound = false; foreach (var property in entityType.GetProperties()) { @@ -2435,6 +2446,14 @@ protected virtual void ValidateMappingFragment( if (!property.IsPrimaryKey()) { propertiesFound = true; + + if (fragment.IsOptional + && !property.IsColumnNullable(fragment.StoreObject)) + { + throw new InvalidOperationException( + RelationalStrings.EntitySplittingNonNullablePropertyOnOptionalFragment( + entityType.DisplayName(), fragment.StoreObject.DisplayName(), property.Name)); + } } } diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index b7a57527cd8..29f755dac4f 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -759,6 +759,22 @@ public static string EntitySplittingMissingRequiredPropertiesOptionalDependent(o GetString("EntitySplittingMissingRequiredPropertiesOptionalDependent", nameof(entityType), nameof(storeObject), nameof(requiredDependentConfig)), entityType, storeObject, requiredDependentConfig); + /// + /// Entity type '{entityType}' has an optional split mapping for '{storeObject}', but the non-nullable property '{property}' is mapped to it. All non-key properties mapped to an optional split fragment must be configured as nullable for '{storeObject}'. + /// + public static string EntitySplittingNonNullablePropertyOnOptionalFragment(object? entityType, object? storeObject, object? property) + => string.Format( + GetString("EntitySplittingNonNullablePropertyOnOptionalFragment", nameof(entityType), nameof(storeObject), nameof(property)), + entityType, storeObject, property); + + /// + /// Entity type '{entityType}' has an optional split mapping for '{storeObject}', but that store object is also shared with entity type '{principalEntityType}' via table splitting. Combining an optional split fragment with table sharing on the same store object is not supported. + /// + public static string EntitySplittingOptionalFragmentSharedTable(object? entityType, object? storeObject, object? principalEntityType) + => string.Format( + GetString("EntitySplittingOptionalFragmentSharedTable", nameof(entityType), nameof(storeObject), nameof(principalEntityType)), + entityType, storeObject, principalEntityType); + /// /// Entity type '{entityType}' has a split mapping for '{storeObject}', but it doesn't have a main mapping of the same type. Map '{entityType}' to '{storeObjectType}'. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index dffc8bf7851..31c67f9be35 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -400,6 +400,12 @@ Entity type '{entityType}' has a split mapping and is an optional dependent sharing a store object, but it doesn't map any required non-shared property to the main store object. Keep at least one required non-shared property mapped to a column on '{storeObject}' or mark '{entityType}' as a required dependent by calling '{requiredDependentConfig}'. + + Entity type '{entityType}' has an optional split mapping for '{storeObject}', but the non-nullable property '{property}' is mapped to it. All non-key properties mapped to an optional split fragment must be configured as nullable for '{storeObject}'. + + + Entity type '{entityType}' has an optional split mapping for '{storeObject}', but that store object is also shared with entity type '{principalEntityType}' via table splitting. Combining an optional split fragment with table sharing on the same store object is not supported. + Entity type '{entityType}' has a split mapping for '{storeObject}', but it doesn't have a main mapping of the same type. Map '{entityType}' to '{storeObjectType}'. diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index 0856a99610f..2bd9d9364f5 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -1575,6 +1575,106 @@ public virtual void Detects_unmapped_foreign_keys_in_entity_splitting() LogLevel.Error); } + [Fact] + public virtual void Optional_entity_splitting_fragment_with_nullable_reference_property_is_valid() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity().SplitToTable( + "AnimalDetails", s => + { + s.IsOptional(); + s.Property(a => a.Name); + }); + + Validate(modelBuilder); + } + + [Fact] + public virtual void Optional_entity_splitting_fragment_with_nullable_value_type_property_is_valid() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity().ToTable("Cats"); + modelBuilder.Entity().SplitToTable( + "CatDetails", s => + { + s.IsOptional(); + s.Property("OptionalIdentity"); + }); + + Validate(modelBuilder); + } + + [Fact] + public virtual void Detects_non_nullable_reference_property_on_optional_entity_splitting_fragment() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity().Property(a => a.Name).IsRequired(); + modelBuilder.Entity().SplitToTable( + "AnimalDetails", s => + { + s.IsOptional(); + s.Property(a => a.Name); + }); + + VerifyError( + RelationalStrings.EntitySplittingNonNullablePropertyOnOptionalFragment( + nameof(Animal), "AnimalDetails", nameof(Animal.Name)), + modelBuilder); + } + + [Fact] + public virtual void Detects_non_nullable_value_type_property_on_optional_entity_splitting_fragment() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity().ToTable("Cats"); + modelBuilder.Entity().SplitToTable( + "CatDetails", s => + { + s.IsOptional(); + s.Property(c => c.Identity); + }); + + VerifyError( + RelationalStrings.EntitySplittingNonNullablePropertyOnOptionalFragment( + nameof(Cat), "CatDetails", nameof(Cat.Identity)), + modelBuilder); + } + + [Fact] + public virtual void Entity_splitting_primary_key_remains_non_nullable_when_fragment_is_optional() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity().SplitToTable( + "AnimalDetails", s => + { + s.IsOptional(); + s.Property(a => a.Id); + s.Property(a => a.Name); + }); + + var model = Validate(modelBuilder); + var entityType = model.FindEntityType(typeof(Animal))!; + var storeObject = StoreObjectIdentifier.Table("AnimalDetails"); + + Assert.False(entityType.FindProperty(nameof(Animal.Id))!.IsColumnNullable(storeObject)); + } + + [Fact] + public virtual void Mixed_required_and_optional_entity_splitting_fragments_are_valid() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity().ToTable("Cats"); + modelBuilder.Entity().SplitToTable("CatRequiredDetails", s => s.Property(c => c.Breed)); + modelBuilder.Entity().SplitToTable( + "CatOptionalDetails", s => + { + s.IsOptional(); + s.Property("OptionalNotes"); + }); + + Validate(modelBuilder); + } + [Fact] public virtual void Detects_duplicate_columns_in_derived_types_with_different_types() { From 1b6f503d8421d97ecb9ea95d38c0c1e86bfc1df9 Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Fri, 31 Jul 2026 17:35:32 +0500 Subject: [PATCH 03/11] Add IsSplitFragmentOptional to table mappings Part of #27974 --- ...nalCSharpRuntimeAnnotationCodeGenerator.cs | 3 ++- .../Metadata/ITableMappingBase.cs | 12 ++++++++++ .../Metadata/Internal/RelationalModel.cs | 24 ++++++++++++------- .../Metadata/Internal/TableMappingBase.cs | 3 +++ 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs b/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs index 5a4094b84b5..7ec22ca8cb1 100644 --- a/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs +++ b/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs @@ -1774,7 +1774,8 @@ private void GenerateAddMapping( .Append($"{tableVariable}, {additionalParameter ?? ""}{code.Literal(tableMapping.IncludesDerivedTypes)}"); if (tableMapping.IsSharedTablePrincipal.HasValue - || tableMapping.IsSplitEntityTypePrincipal.HasValue) + || tableMapping.IsSplitEntityTypePrincipal.HasValue + || tableMapping.IsSplitFragmentOptional) { mainBuilder.AppendLine(")") .AppendLine("{").IncrementIndent(); diff --git a/src/EFCore.Relational/Metadata/ITableMappingBase.cs b/src/EFCore.Relational/Metadata/ITableMappingBase.cs index f3b42f74bee..1b4e50ca8b0 100644 --- a/src/EFCore.Relational/Metadata/ITableMappingBase.cs +++ b/src/EFCore.Relational/Metadata/ITableMappingBase.cs @@ -40,6 +40,13 @@ public interface ITableMappingBase : IAnnotatable /// bool? IsSplitEntityTypePrincipal { get; } + /// + /// Gets the value indicating whether a row for this mapping might not exist even when a row exists in the + /// principal table-like object for the entity type. Always for the principal mapping. + /// + bool IsSplitFragmentOptional + => false; + /// /// Gets the value indicating whether the mapped table-like object includes rows for the derived entity types. /// Set to for inherited mappings. if the entity type has no derived types. @@ -111,6 +118,11 @@ string ToDebugString(MetadataDebugStringOptions options = MetadataDebugStringOpt builder.Append("IsSplitEntityTypePrincipal"); } + if (IsSplitFragmentOptional) + { + builder.Append(" IsSplitFragmentOptional"); + } + if (!singleLine && (options & MetadataDebugStringOptions.IncludeAnnotations) != 0) { builder.Append(AnnotationsToDebugString(indent + 2)); diff --git a/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs b/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs index a387e2c5ed3..4ddde20a3e7 100644 --- a/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs +++ b/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs @@ -463,7 +463,8 @@ private static void AddTables( databaseModel, tableMappings, includesDerivedTypes: includesDerivedTypes, - isSplitEntityTypePrincipal: false); + isSplitEntityTypePrincipal: false, + isSplitFragmentOptional: fragment.IsOptional); } CreateTableMapping( @@ -495,7 +496,8 @@ private static void CreateTableMapping( RelationalModel databaseModel, List tableMappings, bool? includesDerivedTypes, - bool? isSplitEntityTypePrincipal = null) + bool? isSplitEntityTypePrincipal = null, + bool isSplitFragmentOptional = false) { if (!databaseModel.Tables.TryGetValue((mappedTable.Name, mappedTable.Schema), out var table)) { @@ -506,7 +508,8 @@ private static void CreateTableMapping( var tableMapping = new TableMapping(typeBase, table, includesDerivedTypes) { - IsSplitEntityTypePrincipal = isSplitEntityTypePrincipal + IsSplitEntityTypePrincipal = isSplitEntityTypePrincipal, + IsSplitFragmentOptional = isSplitFragmentOptional }; var containerColumnName = mappedType.GetContainerColumnName(mappedTable); @@ -571,7 +574,8 @@ private static void CreateTableMapping( databaseModel, complexTableMappings, includesDerivedTypes: true, - isSplitEntityTypePrincipal: isSplitEntityTypePrincipal == true ? false : isSplitEntityTypePrincipal); + isSplitEntityTypePrincipal: isSplitEntityTypePrincipal == true ? false : isSplitEntityTypePrincipal, + isSplitFragmentOptional: isSplitFragmentOptional); } if (((ITableMappingBase)tableMapping).ColumnMappings.Any() @@ -982,7 +986,8 @@ private static void AddViews( databaseModel, viewMappings, includesDerivedTypes: includesDerivedTypes, - isSplitEntityTypePrincipal: false); + isSplitEntityTypePrincipal: false, + isSplitFragmentOptional: fragment.IsOptional); } CreateViewMapping( @@ -1014,7 +1019,8 @@ private static void CreateViewMapping( RelationalModel databaseModel, List viewMappings, bool? includesDerivedTypes, - bool? isSplitEntityTypePrincipal = null) + bool? isSplitEntityTypePrincipal = null, + bool isSplitFragmentOptional = false) { if (!databaseModel.Views.TryGetValue((mappedView.Name, mappedView.Schema), out var view)) { @@ -1025,7 +1031,8 @@ private static void CreateViewMapping( var viewMapping = new ViewMapping(entityType, view, includesDerivedTypes) { - IsSplitEntityTypePrincipal = isSplitEntityTypePrincipal + IsSplitEntityTypePrincipal = isSplitEntityTypePrincipal, + IsSplitFragmentOptional = isSplitFragmentOptional }; var containerColumnName = mappedType.GetContainerColumnName(mappedView); @@ -1089,7 +1096,8 @@ private static void CreateViewMapping( databaseModel, complexViewMappings, includesDerivedTypes: true, - isSplitEntityTypePrincipal: isSplitEntityTypePrincipal == true ? false : isSplitEntityTypePrincipal); + isSplitEntityTypePrincipal: isSplitEntityTypePrincipal == true ? false : isSplitEntityTypePrincipal, + isSplitFragmentOptional: isSplitFragmentOptional); } if (((ITableMappingBase)viewMapping).ColumnMappings.Any() diff --git a/src/EFCore.Relational/Metadata/Internal/TableMappingBase.cs b/src/EFCore.Relational/Metadata/Internal/TableMappingBase.cs index 7f2f26fe570..68c7e859838 100644 --- a/src/EFCore.Relational/Metadata/Internal/TableMappingBase.cs +++ b/src/EFCore.Relational/Metadata/Internal/TableMappingBase.cs @@ -85,6 +85,9 @@ public virtual bool AddColumnMapping(TColumnMapping columnMapping) /// public virtual bool? IsSplitEntityTypePrincipal { get; init; } + /// + public virtual bool IsSplitFragmentOptional { get; init; } + /// /// 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 From 4bdb7ebcc0dacb921a989dcaa88dde1c62692d7c Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Fri, 31 Jul 2026 17:37:00 +0500 Subject: [PATCH 04/11] Use LEFT JOIN for optional entity-splitting fragments Part of #27974 --- ...anslatingExpressionVisitor.CreateSelect.cs | 15 +- .../OptionalEntitySplittingQuerySqliteTest.cs | 177 ++++++++++++++++++ 2 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 test/EFCore.Sqlite.FunctionalTests/Query/OptionalEntitySplittingQuerySqliteTest.cs diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.CreateSelect.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.CreateSelect.cs index 787434ed986..34ffabce827 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.CreateSelect.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.CreateSelect.cs @@ -410,7 +410,10 @@ Expression ProcessComplexPropertyTpc( .Zip(innerColumns, _sqlExpressionFactory.Equal) .Aggregate(_sqlExpressionFactory.AndAlso); - tables.Add(new InnerJoinExpression(tableExpression, joinPredicate, prunable: true)); + tables.Add( + mapping.IsSplitFragmentOptional + ? new LeftJoinExpression(tableExpression, joinPredicate, prunable: true) + : new InnerJoinExpression(tableExpression, joinPredicate, prunable: true)); } } @@ -421,15 +424,19 @@ Expression ProcessComplexPropertyTpc( continue; } - var columnBase = mappings.Select(e => e.Table.FindColumn(property)).First(e => e != null)!; - propertyMap[property] = CreateColumnExpression(property, columnBase, tableMap[columnBase.Table], nullable: false); + var mapping = mappings.First(e => e.Table.FindColumn(property) != null); + var columnBase = mapping.Table.FindColumn(property)!; + propertyMap[property] = CreateColumnExpression( + property, columnBase, tableMap[columnBase.Table], nullable: mapping.IsSplitFragmentOptional); } var complexPropertyMap = new Dictionary(); foreach (var complexProperty in entityType.GetComplexProperties()) { var table = FindTable(complexProperty, mappings); - complexPropertyMap[complexProperty] = ProcessComplexProperty(complexProperty, table, tableMap[table], containerNullable: false); + var containerNullable = mappings.First(m => m.Table == table).IsSplitFragmentOptional; + complexPropertyMap[complexProperty] = + ProcessComplexProperty(complexProperty, table, tableMap[table], containerNullable: containerNullable); } var projection = new StructuralTypeProjectionExpression(entityType, propertyMap, complexPropertyMap, tableMap: tableMap); diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/OptionalEntitySplittingQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/OptionalEntitySplittingQuerySqliteTest.cs new file mode 100644 index 00000000000..f3a6a0f3201 --- /dev/null +++ b/test/EFCore.Sqlite.FunctionalTests/Query/OptionalEntitySplittingQuerySqliteTest.cs @@ -0,0 +1,177 @@ +// 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.Query; + +#nullable disable + +public class OptionalEntitySplittingQuerySqliteTest : NonSharedModelTestBase, IClassFixture +{ + public OptionalEntitySplittingQuerySqliteTest(NonSharedFixture fixture) + : base(fixture) + { + } + + protected override string NonSharedStoreName + => "OptionalEntitySplittingQueryTest"; + + protected override ITestStoreFactory NonSharedTestStoreFactory + => SqliteTestStoreFactory.Instance; + + private static void OnModelCreating(ModelBuilder modelBuilder) + => modelBuilder.Entity(b => + { + b.ToTable("Customers"); + b.Property(c => c.Id).ValueGeneratedNever(); + + // A required fragment: always joined with INNER JOIN. + b.SplitToTable("CustomerStats", t => t.Property(c => c.VisitCount)); + + // An optional fragment: joined with LEFT JOIN, row may be absent. + b.SplitToTable( + "CustomerDetails", t => + { + t.IsOptional(); + t.Property(c => c.Description); + t.Property(c => c.Score); + }); + }); + + private static async Task Seed(CustomerContext context) + { + context.Customers.AddRange( + new Customer { Id = 1, Name = "Alice", VisitCount = 5 }, + new Customer { Id = 2, Name = "Bob", VisitCount = 3 }, + new Customer { Id = 3, Name = "Carol", VisitCount = 1 }); + await context.SaveChangesAsync(); + + // Customer 1: optional fragment row present, with non-null values. + await context.Database.ExecuteSqlRawAsync( + "INSERT INTO CustomerDetails (Id, Description, Score) VALUES (1, 'Has details', 42)"); + + // Customer 2: optional fragment row absent entirely. + + // Customer 3: optional fragment row present, but its payload is null. + await context.Database.ExecuteSqlRawAsync( + "INSERT INTO CustomerDetails (Id, Description, Score) VALUES (3, NULL, NULL)"); + } + + private async Task> InitializeContextAsync() + => await InitializeNonSharedTest(OnModelCreating, seed: Seed, shouldLogCategory: _ => true); + + [Fact] + public async Task Missing_optional_fragment_row_still_returns_entity() + { + var contextFactory = await InitializeContextAsync(); + await using var context = contextFactory.CreateDbContext(); + + var customers = await context.Customers.OrderBy(c => c.Id).ToListAsync(); + + Assert.Equal(3, customers.Count); + + Assert.Equal("Has details", customers[0].Description); + Assert.Equal(42, customers[0].Score); + Assert.Equal(5, customers[0].VisitCount); + + // Absent row: entity still materializes, missing columns are null. + Assert.Null(customers[1].Description); + Assert.Null(customers[1].Score); + Assert.Equal(3, customers[1].VisitCount); + + // Existing row with null payload: same shape as absent row. + Assert.Null(customers[2].Description); + Assert.Null(customers[2].Score); + Assert.Equal(1, customers[2].VisitCount); + } + + [Fact] + public async Task Required_fragment_uses_inner_join_and_optional_fragment_uses_left_join() + { + var contextFactory = await InitializeContextAsync(); + await using var context = contextFactory.CreateDbContext(); + + var queryString = context.Customers.ToQueryString(); + + Assert.Contains("INNER JOIN \"CustomerStats\"", queryString); + Assert.Contains("LEFT JOIN \"CustomerDetails\"", queryString); + } + + [Fact] + public async Task Predicate_on_optional_fragment_value() + { + var contextFactory = await InitializeContextAsync(); + await using var context = contextFactory.CreateDbContext(); + + var customers = await context.Customers.Where(c => c.Description == "Has details").ToListAsync(); + + Assert.Equal([1], customers.Select(c => c.Id)); + } + + [Fact] + public async Task Predicate_comparing_optional_fragment_property_to_null_matches_absent_and_null_row() + { + var contextFactory = await InitializeContextAsync(); + await using var context = contextFactory.CreateDbContext(); + + var customers = await context.Customers.Where(c => c.Description == null).OrderBy(c => c.Id).ToListAsync(); + + Assert.Equal([2, 3], customers.Select(c => c.Id)); + } + + [Fact] + public async Task Projection_containing_optional_fragment_properties() + { + var contextFactory = await InitializeContextAsync(); + await using var context = contextFactory.CreateDbContext(); + + var results = await context.Customers.OrderBy(c => c.Id) + .Select(c => new { c.Id, c.Description, c.Score }) + .ToListAsync(); + + Assert.Equal(3, results.Count); + Assert.Equal("Has details", results[0].Description); + Assert.Null(results[1].Description); + Assert.Null(results[2].Description); + } + + [Fact] + public async Task NoTracking_query_materializes_optional_fragment_correctly() + { + var contextFactory = await InitializeContextAsync(); + await using var context = contextFactory.CreateDbContext(); + + var customers = await context.Customers.AsNoTracking().OrderBy(c => c.Id).ToListAsync(); + + Assert.Equal("Has details", customers[0].Description); + Assert.Null(customers[1].Description); + Assert.Null(customers[2].Description); + + Assert.All(customers, c => Assert.Equal(EntityState.Detached, context.Entry(c).State)); + } + + [Fact] + public async Task Tracking_query_materializes_optional_fragment_correctly() + { + var contextFactory = await InitializeContextAsync(); + await using var context = contextFactory.CreateDbContext(); + + var customers = await context.Customers.OrderBy(c => c.Id).ToListAsync(); + + Assert.All(customers, c => Assert.Equal(EntityState.Unchanged, context.Entry(c).State)); + Assert.Null(customers[1].Description); + } + + protected class CustomerContext(DbContextOptions options) : PoolableDbContext(options) + { + public DbSet Customers { get; set; } + } + + protected class Customer + { + public int Id { get; set; } + public string Name { get; set; } + public int VisitCount { get; set; } + public string Description { get; set; } + public int? Score { get; set; } + } +} From 7eb77b4b10c2f915dd249b8b7a22e4e5f1c95305 Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Fri, 31 Jul 2026 17:38:58 +0500 Subject: [PATCH 05/11] Support saving optional entity-splitting fragments on SQL Server Part of #27974 --- .../Properties/RelationalStrings.Designer.cs | 6 + .../Properties/RelationalStrings.resx | 3 + .../Update/IUpdateSqlGenerator.cs | 32 +++ .../Update/Internal/CommandBatchPreparer.cs | 7 + .../Update/ModificationCommand.cs | 9 + .../Update/ReaderModificationCommandBatch.cs | 16 +- .../Update/UpdateSqlGenerator.cs | 16 ++ .../Internal/SqlServerUpdateSqlGenerator.cs | 212 +++++++++++++++++ .../Update/SqlServerUpdateSqlGeneratorTest.cs | 218 ++++++++++++++++++ .../OptionalEntitySplittingSqliteTest.cs | 106 +++++++++ 10 files changed, 621 insertions(+), 4 deletions(-) create mode 100644 test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 29f755dac4f..dd510b1e2d9 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -1805,6 +1805,12 @@ public static string OptionalDependentWithDependentWithoutIdentifyingProperty(ob GetString("OptionalDependentWithDependentWithoutIdentifyingProperty", nameof(entityType)), entityType); + /// + /// Saving changes to an optional entity-splitting fragment is not supported by the current database provider. Either configure the fragment as required by removing the call to 'IsOptional()', or use a provider that supports optional entity-splitting fragments for 'SaveChanges'. + /// + public static string OptionalEntitySplittingNotSupported + => GetString("OptionalEntitySplittingNotSupported"); + /// /// The value provided for parameter '{parameter}' cannot be used because it isn't assignable to type 'object[]'. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 31c67f9be35..ea0578deee7 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1146,6 +1146,9 @@ Entity type '{entityType}' is an optional dependent using table sharing and containing other dependents without any required non shared property to identify whether the entity exists. If all nullable properties contain a 'null' value in database then an object instance won't be created in the query causing nested dependent's values to be lost. Add a required property to create instances with 'null' values for other properties or mark the incoming navigation as required to always create an instance. + + Saving changes to an optional entity-splitting fragment is not supported by the current database provider. Either configure the fragment as required by removing the call to 'IsOptional()', or use a provider that supports optional entity-splitting fragments for 'SaveChanges'. + The value provided for parameter '{parameter}' cannot be used because it isn't assignable to type 'object[]'. diff --git a/src/EFCore.Relational/Update/IUpdateSqlGenerator.cs b/src/EFCore.Relational/Update/IUpdateSqlGenerator.cs index aca1270f895..5a7842162d8 100644 --- a/src/EFCore.Relational/Update/IUpdateSqlGenerator.cs +++ b/src/EFCore.Relational/Update/IUpdateSqlGenerator.cs @@ -174,4 +174,36 @@ ResultSetMapping AppendStoredProcedureCall( IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction); + + /// + /// Appends SQL that atomically updates the row for an optional entity-splitting fragment if it exists, inserts it if it + /// doesn't and at least one non-key value being written is non-, or leaves the row absent if it + /// doesn't exist and every non-key value being written is . + /// + /// The builder to which the SQL should be appended. + /// The command that represents the update operation. + /// The ordinal of this command in the batch. + /// Returns whether the SQL appended must be executed in a transaction to work correctly. + /// The for the command. + ResultSetMapping AppendOptionalFragmentUpsertOperation( + StringBuilder commandStringBuilder, + IReadOnlyModificationCommand command, + int commandPosition, + out bool requiresTransaction) + => throw new NotSupportedException(RelationalStrings.OptionalEntitySplittingNotSupported); + + /// + /// Appends SQL that deletes the row for an optional entity-splitting fragment, tolerating the row already being absent. + /// + /// The builder to which the SQL should be appended. + /// The command that represents the delete operation. + /// The ordinal of this command in the batch. + /// Returns whether the SQL appended must be executed in a transaction to work correctly. + /// The for the command. + ResultSetMapping AppendOptionalFragmentDeleteOperation( + StringBuilder commandStringBuilder, + IReadOnlyModificationCommand command, + int commandPosition, + out bool requiresTransaction) + => throw new NotSupportedException(RelationalStrings.OptionalEntitySplittingNotSupported); } diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 8d411abb2de..a86d8580484 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -123,6 +123,13 @@ private IEnumerable CreateCommandBatches( continue; } + if (modificationCommand.EntityState == EntityState.Added + && modificationCommand is ModificationCommand { IsOptionalSplitFragment: true } + && modificationCommand.ColumnModifications.Where(m => !m.IsKey).All(m => m.Value is null)) + { + continue; + } + if (!batch.TryAddCommand(modificationCommand)) { if (batch.ModificationCommands.Count == 1 diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs index 82f70b14103..301712a20ad 100644 --- a/src/EFCore.Relational/Update/ModificationCommand.cs +++ b/src/EFCore.Relational/Update/ModificationCommand.cs @@ -94,6 +94,15 @@ public virtual EntityState EntityState /// public virtual IColumnBase? RowsAffectedColumn { get; private set; } + /// + /// Gets a value indicating whether this command targets an optional entity-splitting fragment table, i.e. a table + /// for which a row is only expected to exist when at least one non-key property mapped to it is non-. + /// + public virtual bool IsOptionalSplitFragment + => StoreStoredProcedure is null + && _entries.Count > 0 + && GetTableMapping(_entries[0].EntityType) is { IsSplitFragmentOptional: true }; + /// /// The list of needed to perform the insert, update, or delete. /// diff --git a/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs b/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs index b85778783ac..5f7f0a98d19 100644 --- a/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs +++ b/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs @@ -204,6 +204,8 @@ protected virtual void AddCommand(IReadOnlyModificationCommand modificationComma } else { + var isOptionalSplitFragment = modificationCommand is ModificationCommand { IsOptionalSplitFragment: true }; + switch (modificationCommand.EntityState) { case EntityState.Added: @@ -213,13 +215,19 @@ protected virtual void AddCommand(IReadOnlyModificationCommand modificationComma break; case EntityState.Modified: ResultSetMappings.Add( - UpdateSqlGenerator.AppendUpdateOperation( - SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); + isOptionalSplitFragment + ? UpdateSqlGenerator.AppendOptionalFragmentUpsertOperation( + SqlBuilder, modificationCommand, commandPosition, out requiresTransaction) + : UpdateSqlGenerator.AppendUpdateOperation( + SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); break; case EntityState.Deleted: ResultSetMappings.Add( - UpdateSqlGenerator.AppendDeleteOperation( - SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); + isOptionalSplitFragment + ? UpdateSqlGenerator.AppendOptionalFragmentDeleteOperation( + SqlBuilder, modificationCommand, commandPosition, out requiresTransaction) + : UpdateSqlGenerator.AppendDeleteOperation( + SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); break; default: diff --git a/src/EFCore.Relational/Update/UpdateSqlGenerator.cs b/src/EFCore.Relational/Update/UpdateSqlGenerator.cs index a1b5ac922da..3335031fe7c 100644 --- a/src/EFCore.Relational/Update/UpdateSqlGenerator.cs +++ b/src/EFCore.Relational/Update/UpdateSqlGenerator.cs @@ -433,6 +433,22 @@ public virtual ResultSetMapping AppendStoredProcedureCall( return resultSetMapping; } + /// + public virtual ResultSetMapping AppendOptionalFragmentUpsertOperation( + StringBuilder commandStringBuilder, + IReadOnlyModificationCommand command, + int commandPosition, + out bool requiresTransaction) + => throw new NotSupportedException(RelationalStrings.OptionalEntitySplittingNotSupported); + + /// + public virtual ResultSetMapping AppendOptionalFragmentDeleteOperation( + StringBuilder commandStringBuilder, + IReadOnlyModificationCommand command, + int commandPosition, + out bool requiresTransaction) + => throw new NotSupportedException(RelationalStrings.OptionalEntitySplittingNotSupported); + /// /// Appends a SQL fragment for a VALUES. /// diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs index 1999f30bd98..cb72c0de12d 100644 --- a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs +++ b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs @@ -217,6 +217,218 @@ protected override void AppendDeleteCommand( commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); } + /// + /// 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. + /// + /// + /// Generates an UPDATE and, only when it affects no rows and at least one non-key value being written is + /// non-, a follow-up INSERT: the row for an optional entity-splitting fragment is + /// created lazily and is atomically updated or inserted within a single round trip, without using MERGE. + /// A row-count mismatch after the UPDATE that isn't explained by the row simply being absent is treated as + /// a genuine concurrency conflict and reported through the normal affected-count consumption pipeline. + /// + public override ResultSetMapping AppendOptionalFragmentUpsertOperation( + StringBuilder commandStringBuilder, + IReadOnlyModificationCommand command, + int commandPosition, + out bool requiresTransaction) + { + var name = command.TableName; + var schema = command.Schema; + var operations = command.ColumnModifications; + + var writeOperations = operations.Where(o => o.IsWrite).ToList(); + var keyOperations = operations.Where(o => o.IsKey).ToList(); + var conditionOperations = operations.Where(o => o.IsCondition).ToList(); + var tokenOperations = conditionOperations.Where(o => !o.IsKey).ToList(); + var readOperations = operations.Where(o => o.IsRead).ToList(); + + if (readOperations.Count > 0) + { + throw new NotSupportedException(RelationalStrings.OptionalEntitySplittingNotSupported); + } + + requiresTransaction = true; + + var rowsAffectedVariable = "@_fragmentRowsAffected" + commandPosition.ToString(CultureInfo.InvariantCulture); + + commandStringBuilder + .Append("DECLARE ").Append(rowsAffectedVariable).Append(" int") + .AppendLine(SqlGenerationHelper.StatementTerminator) + .AppendLine(); + + AppendUpdateCommandHeader(commandStringBuilder, name, schema, writeOperations); + AppendWhereClause(commandStringBuilder, conditionOperations); + commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); + + commandStringBuilder + .Append("SET ").Append(rowsAffectedVariable).Append(" = @@ROWCOUNT") + .AppendLine(SqlGenerationHelper.StatementTerminator) + .AppendLine(); + + commandStringBuilder + .Append("IF ").Append(rowsAffectedVariable).AppendLine(" = 0") + .AppendLine("BEGIN"); + + var indent = " "; + if (tokenOperations.Count > 0) + { + // A non-key (concurrency token) condition narrowed the UPDATE. Zero rows affected could mean either that the + // row doesn't exist (fine, no conflict) or that it exists but the token didn't match (a genuine conflict). + commandStringBuilder.Append(indent).Append("IF NOT EXISTS (SELECT 1 FROM "); + SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, name, schema); + commandStringBuilder + .Append(" WHERE ") + .AppendJoin(keyOperations, (sb, o) => AppendWhereCondition(sb, o, useOriginalValue: true), " AND ") + .AppendLine(")") + .Append(indent).AppendLine("BEGIN"); + indent += " "; + } + + commandStringBuilder.Append(indent).Append("IF (") + .AppendJoin( + writeOperations, (sb, o) => + { + AppendParameterOrLiteral(sb, o, useOriginal: false); + sb.Append(" IS NOT NULL"); + }, " OR ") + .AppendLine(")") + .Append(indent).AppendLine("BEGIN"); + + var insertColumns = keyOperations.Concat(writeOperations).ToList(); + commandStringBuilder.Append(indent).Append(" INSERT INTO "); + SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, name, schema); + commandStringBuilder + .Append(" (") + .AppendJoin(insertColumns, SqlGenerationHelper, (sb, o, helper) => helper.DelimitIdentifier(sb, o.ColumnName)) + .AppendLine(")") + .Append(indent).Append(" VALUES (") + .AppendJoin( + keyOperations, (sb, o) => AppendParameterOrLiteral(sb, o, useOriginal: true), ", ") + .Append(keyOperations.Count > 0 && writeOperations.Count > 0 ? ", " : "") + .AppendJoin(writeOperations, (sb, o) => AppendParameterOrLiteral(sb, o, useOriginal: false), ", ") + .Append(")") + .AppendLine(SqlGenerationHelper.StatementTerminator) + .Append(indent).Append(" SET ").Append(rowsAffectedVariable).Append(" = @@ROWCOUNT") + .AppendLine(SqlGenerationHelper.StatementTerminator) + .Append(indent).AppendLine("END") + .Append(indent).AppendLine("ELSE") + .Append(indent).AppendLine("BEGIN") + .Append(indent).Append(" SET ").Append(rowsAffectedVariable).Append(" = 1") + .AppendLine(SqlGenerationHelper.StatementTerminator) + .Append(indent).AppendLine("END"); + + if (tokenOperations.Count > 0) + { + commandStringBuilder.AppendLine(" END"); + } + + commandStringBuilder.AppendLine("END").AppendLine(); + + commandStringBuilder + .Append("SELECT ").Append(rowsAffectedVariable) + .AppendLine(SqlGenerationHelper.StatementTerminator) + .AppendLine(); + + return ResultSetMapping.LastInResultSet | ResultSetMapping.ResultSetWithRowsAffectedOnly; + } + + /// + /// 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. + /// + /// + /// Generates a DELETE for an optional entity-splitting fragment that tolerates the row already being + /// absent: zero rows affected is only reported as a genuine concurrency conflict when a concurrency token + /// condition narrowed the DELETE and a row with the given key still exists. + /// + public override ResultSetMapping AppendOptionalFragmentDeleteOperation( + StringBuilder commandStringBuilder, + IReadOnlyModificationCommand command, + int commandPosition, + out bool requiresTransaction) + { + var name = command.TableName; + var schema = command.Schema; + var operations = command.ColumnModifications; + + var keyOperations = operations.Where(o => o.IsKey).ToList(); + var conditionOperations = operations.Where(o => o.IsCondition).ToList(); + var tokenOperations = conditionOperations.Where(o => !o.IsKey).ToList(); + + requiresTransaction = true; + + var rowsAffectedVariable = "@_fragmentRowsAffected" + commandPosition.ToString(CultureInfo.InvariantCulture); + + commandStringBuilder + .Append("DECLARE ").Append(rowsAffectedVariable).Append(" int") + .AppendLine(SqlGenerationHelper.StatementTerminator) + .AppendLine(); + + commandStringBuilder.Append("DELETE FROM "); + SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, name, schema); + AppendWhereClause(commandStringBuilder, conditionOperations); + commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); + + commandStringBuilder + .Append("SET ").Append(rowsAffectedVariable).Append(" = @@ROWCOUNT") + .AppendLine(SqlGenerationHelper.StatementTerminator) + .AppendLine(); + + commandStringBuilder + .Append("IF ").Append(rowsAffectedVariable).AppendLine(" = 0") + .AppendLine("BEGIN"); + + if (tokenOperations.Count > 0) + { + // The DELETE's concurrency-token condition could be why no rows were affected. Treat the row as tolerably + // absent only if a row with the key genuinely doesn't exist; otherwise this is a real concurrency conflict. + commandStringBuilder.Append(" IF NOT EXISTS (SELECT 1 FROM "); + SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, name, schema); + commandStringBuilder + .Append(" WHERE ") + .AppendJoin(keyOperations, (sb, o) => AppendWhereCondition(sb, o, useOriginalValue: true), " AND ") + .AppendLine(")") + .AppendLine(" BEGIN") + .Append(" SET ").Append(rowsAffectedVariable).Append(" = 1") + .AppendLine(SqlGenerationHelper.StatementTerminator) + .AppendLine(" END"); + } + else + { + commandStringBuilder + .Append(" SET ").Append(rowsAffectedVariable).Append(" = 1") + .AppendLine(SqlGenerationHelper.StatementTerminator); + } + + commandStringBuilder.AppendLine("END").AppendLine(); + + commandStringBuilder + .Append("SELECT ").Append(rowsAffectedVariable) + .AppendLine(SqlGenerationHelper.StatementTerminator) + .AppendLine(); + + return ResultSetMapping.LastInResultSet | ResultSetMapping.ResultSetWithRowsAffectedOnly; + } + + private void AppendParameterOrLiteral(StringBuilder commandStringBuilder, IColumnModification columnModification, bool useOriginal) + { + if (useOriginal ? columnModification.UseOriginalValueParameter : columnModification.UseCurrentValueParameter) + { + SqlGenerationHelper.GenerateParameterNamePlaceholder( + commandStringBuilder, useOriginal ? columnModification.OriginalParameterName! : columnModification.ParameterName!); + } + else + { + AppendSqlLiteral(commandStringBuilder, columnModification, null, null); + } + } + /// /// 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 diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs index 9d4c06e9d0f..dc1bae35304 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs @@ -1,9 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Update.Internal; +using Microsoft.EntityFrameworkCore.Update.Internal; // ReSharper disable InconsistentNaming namespace Microsoft.EntityFrameworkCore.Update; @@ -243,4 +245,220 @@ protected override string CloseDelimiter private void AssertBaseline(string expected, string actual) => Assert.Equal(expected, actual.TrimEnd(), ignoreLineEndingDifferences: true); + + [Fact] + public void AppendOptionalFragmentUpsertOperation_generates_update_then_conditional_insert() + { + var stringBuilder = new StringBuilder(); + var command = CreateOptionalFragmentCommand(EntityState.Modified, concurrencyToken: false); + + CreateSqlGenerator().AppendOptionalFragmentUpsertOperation(stringBuilder, command, 0, out var requiresTransaction); + + AssertBaseline( + """ +DECLARE @_fragmentRowsAffected0 int; + +UPDATE [dbo].[CustomerDetails] SET [Description] = @p0, [Score] = @p1 +WHERE [Id] = @p2; +SET @_fragmentRowsAffected0 = @@ROWCOUNT; + +IF @_fragmentRowsAffected0 = 0 +BEGIN + IF (@p0 IS NOT NULL OR @p1 IS NOT NULL) + BEGIN + INSERT INTO [dbo].[CustomerDetails] ([Id], [Description], [Score]) + VALUES (@p2, @p0, @p1); + SET @_fragmentRowsAffected0 = @@ROWCOUNT; + END + ELSE + BEGIN + SET @_fragmentRowsAffected0 = 1; + END +END + +SELECT @_fragmentRowsAffected0; +""", + stringBuilder.ToString()); + Assert.True(requiresTransaction); + } + + [Fact] + public void AppendOptionalFragmentUpsertOperation_distinguishes_absence_from_conflict_with_concurrency_token() + { + var stringBuilder = new StringBuilder(); + var command = CreateOptionalFragmentCommand(EntityState.Modified, concurrencyToken: true); + + CreateSqlGenerator().AppendOptionalFragmentUpsertOperation(stringBuilder, command, 0, out var requiresTransaction); + + AssertBaseline( + """ +DECLARE @_fragmentRowsAffected0 int; + +UPDATE [dbo].[CustomerDetails] SET [Description] = @p0, [Score] = @p1 +WHERE [Id] = @p2 AND [Version] IS NULL; +SET @_fragmentRowsAffected0 = @@ROWCOUNT; + +IF @_fragmentRowsAffected0 = 0 +BEGIN + IF NOT EXISTS (SELECT 1 FROM [dbo].[CustomerDetails] WHERE [Id] = @p2) + BEGIN + IF (@p0 IS NOT NULL OR @p1 IS NOT NULL) + BEGIN + INSERT INTO [dbo].[CustomerDetails] ([Id], [Description], [Score]) + VALUES (@p2, @p0, @p1); + SET @_fragmentRowsAffected0 = @@ROWCOUNT; + END + ELSE + BEGIN + SET @_fragmentRowsAffected0 = 1; + END + END +END + +SELECT @_fragmentRowsAffected0; +""", + stringBuilder.ToString()); + Assert.True(requiresTransaction); + } + + [Fact] + public void AppendOptionalFragmentDeleteOperation_generates_tolerant_delete() + { + var stringBuilder = new StringBuilder(); + var command = CreateOptionalFragmentCommand(EntityState.Deleted, concurrencyToken: false); + + CreateSqlGenerator().AppendOptionalFragmentDeleteOperation(stringBuilder, command, 0, out var requiresTransaction); + + AssertBaseline( + """ +DECLARE @_fragmentRowsAffected0 int; + +DELETE FROM [dbo].[CustomerDetails] +WHERE [Id] = @p0; +SET @_fragmentRowsAffected0 = @@ROWCOUNT; + +IF @_fragmentRowsAffected0 = 0 +BEGIN + SET @_fragmentRowsAffected0 = 1; +END + +SELECT @_fragmentRowsAffected0; +""", + stringBuilder.ToString()); + Assert.True(requiresTransaction); + } + + [Fact] + public void AppendOptionalFragmentDeleteOperation_distinguishes_absence_from_conflict_with_concurrency_token() + { + var stringBuilder = new StringBuilder(); + var command = CreateOptionalFragmentCommand(EntityState.Deleted, concurrencyToken: true); + + CreateSqlGenerator().AppendOptionalFragmentDeleteOperation(stringBuilder, command, 0, out var requiresTransaction); + + AssertBaseline( + """ +DECLARE @_fragmentRowsAffected0 int; + +DELETE FROM [dbo].[CustomerDetails] +WHERE [Id] = @p0 AND [Version] IS NULL; +SET @_fragmentRowsAffected0 = @@ROWCOUNT; + +IF @_fragmentRowsAffected0 = 0 +BEGIN + IF NOT EXISTS (SELECT 1 FROM [dbo].[CustomerDetails] WHERE [Id] = @p0) + BEGIN + SET @_fragmentRowsAffected0 = 1; + END +END + +SELECT @_fragmentRowsAffected0; +""", + stringBuilder.ToString()); + Assert.True(requiresTransaction); + } + + [Fact] + public void AppendOptionalFragmentUpsertOperation_throws_for_generated_values() + { + var stringBuilder = new StringBuilder(); + var command = CreateOptionalFragmentCommand(EntityState.Modified, concurrencyToken: false, generatedValue: true); + + Assert.Throws( + () => CreateSqlGenerator().AppendOptionalFragmentUpsertOperation(stringBuilder, command, 0, out _)); + } + + private IModificationCommand CreateOptionalFragmentCommand( + EntityState entityState, + bool concurrencyToken, + bool generatedValue = false) + { + var model = GetOptionalFragmentModel(); + var stateManager = TestHelpers.CreateContextServices(model).GetRequiredService(); + var entry = stateManager.GetOrCreateEntry(new OptionalFragmentDetail()); + entry.SetEntityState(entityState); + var generator = new ParameterNameGenerator(); + + var detailType = entry.EntityType; + var idProperty = detailType.FindProperty(nameof(OptionalFragmentDetail.Id)); + var descriptionProperty = detailType.FindProperty(nameof(OptionalFragmentDetail.Description)); + var scoreProperty = detailType.FindProperty(nameof(OptionalFragmentDetail.Score)); + var versionProperty = detailType.FindProperty(nameof(OptionalFragmentDetail.Version)); + + var columnModifications = new List + { + new( + entry, idProperty, idProperty.GetTableColumnMappings().Single().Column, generator.GenerateNext, + idProperty.GetTableColumnMappings().Single().TypeMapping, false, false, true, true, true) + }; + + if (entityState != EntityState.Deleted) + { + columnModifications.Add( + new( + entry, descriptionProperty, descriptionProperty.GetTableColumnMappings().Single().Column, generator.GenerateNext, + descriptionProperty.GetTableColumnMappings().Single().TypeMapping, false, true, false, false, true)); + columnModifications.Add( + new( + entry, scoreProperty, scoreProperty.GetTableColumnMappings().Single().Column, generator.GenerateNext, + scoreProperty.GetTableColumnMappings().Single().TypeMapping, generatedValue, !generatedValue, false, false, true)); + } + + if (concurrencyToken) + { + columnModifications.Add( + new( + entry, versionProperty, versionProperty.GetTableColumnMappings().Single().Column, generator.GenerateNext, + versionProperty.GetTableColumnMappings().Single().TypeMapping, false, false, false, true, true)); + } + + var modificationCommandParameters = new ModificationCommandParameters( + entry.EntityType.GetTableMappings().Single().Table, sensitiveLoggingEnabled: false); + var modificationCommand = CreateMutableModificationCommandFactory().CreateModificationCommand(modificationCommandParameters); + + modificationCommand.AddEntry(entry, mainEntry: true); + + foreach (var columnModification in columnModifications) + { + ((INonTrackedModificationCommand)modificationCommand).AddColumnModification(columnModification); + } + + return modificationCommand; + } + + private IModel GetOptionalFragmentModel() + { + var modelBuilder = TestHelpers.CreateConventionBuilder(); + modelBuilder.Entity().ToTable("CustomerDetails", Schema) + .Property(e => e.Id).ValueGeneratedNever(); + return modelBuilder.Model.FinalizeModel(); + } + + private class OptionalFragmentDetail + { + public int Id { get; set; } + public string Description { get; set; } + public int? Score { get; set; } + public int? Version { get; set; } + } } diff --git a/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs new file mode 100644 index 00000000000..7188b63c2d1 --- /dev/null +++ b/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs @@ -0,0 +1,106 @@ +// 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; + +#nullable disable + +// Sqlite doesn't implement the conditional upsert/delete required to write optional entity-splitting fragments; +// these tests verify that this is surfaced as a clear provider-capability error rather than silently wrong SQL, +// and that operations which never need to touch the optional fragment (e.g. inserting/deleting an all-null +// fragment) keep working, since the "should we touch this fragment at all" decision is provider-neutral. +public class OptionalEntitySplittingSqliteTest : NonSharedModelTestBase, IClassFixture +{ + public OptionalEntitySplittingSqliteTest(NonSharedFixture fixture) + : base(fixture) + { + } + + protected override string NonSharedStoreName + => "OptionalEntitySplittingSaveChangesTest"; + + protected override ITestStoreFactory NonSharedTestStoreFactory + => SqliteTestStoreFactory.Instance; + + private static void OnModelCreating(ModelBuilder modelBuilder) + => modelBuilder.Entity(b => + { + b.ToTable("Customers"); + b.Property(c => c.Id).ValueGeneratedNever(); + b.SplitToTable( + "CustomerDetails", t => + { + t.IsOptional(); + t.Property(c => c.Description); + }); + }); + + private async Task> InitializeContextAsync() + => await InitializeNonSharedTest(OnModelCreating); + + [Fact] + public async Task Insert_with_all_null_optional_payload_does_not_touch_fragment_table() + { + var contextFactory = await InitializeContextAsync(); + + await using (var context = contextFactory.CreateDbContext()) + { + context.Customers.Add(new Customer { Id = 1, Name = "Alice" }); + + // No exception: the optional-fragment INSERT is skipped entirely (provider-neutral decision) + // because every payload value mapped to it is null. + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + Assert.Equal("Alice", customer.Name); + Assert.Null(customer.Description); + } + } + + [Fact] + public async Task Update_of_optional_fragment_throws_provider_capability_error() + { + var contextFactory = await InitializeContextAsync(); + + await using var context = contextFactory.CreateDbContext(); + context.Customers.Add(new Customer { Id = 1, Name = "Alice" }); + await context.SaveChangesAsync(); + + var customer = await context.Customers.SingleAsync(); + customer.Description = "Some details"; + + var exception = await Assert.ThrowsAsync(() => context.SaveChangesAsync()); + Assert.Equal(RelationalStrings.OptionalEntitySplittingNotSupported, exception.Message); + } + + [Fact] + public async Task Delete_of_entity_with_optional_fragment_throws_provider_capability_error() + { + var contextFactory = await InitializeContextAsync(); + + await using var context = contextFactory.CreateDbContext(); + context.Customers.Add(new Customer { Id = 1, Name = "Alice" }); + await context.SaveChangesAsync(); + + var customer = await context.Customers.SingleAsync(); + context.Customers.Remove(customer); + + var exception = await Assert.ThrowsAsync(() => context.SaveChangesAsync()); + Assert.Equal(RelationalStrings.OptionalEntitySplittingNotSupported, exception.Message); + } + + protected class CustomerContext(DbContextOptions options) : PoolableDbContext(options) + { + public DbSet Customers { get; set; } + } + + protected class Customer + { + public int Id { get; set; } + public string Name { get; set; } + public string Description { get; set; } + } +} From b53f4bdb4a45d9fd91d247c445e9858cc0bbdc27 Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Fri, 31 Jul 2026 17:40:46 +0500 Subject: [PATCH 06/11] Emit IsOptional() in compiled models and snapshots Part of #27974 --- .../Migrations/Design/CSharpSnapshotGenerator.cs | 5 +++++ ...RelationalCSharpRuntimeAnnotationCodeGenerator.cs | 12 ++++++++++-- .../Conventions/RelationalRuntimeModelConvention.cs | 3 ++- .../Baselines/BigModel/OwnedTypeEntityType.cs | 3 ++- .../Baselines/No_NativeAOT/OwnedTypeEntityType.cs | 3 ++- .../Baselines/BigModel/OwnedTypeEntityType.cs | 3 ++- .../Baselines/No_NativeAOT/OwnedTypeEntityType.cs | 3 ++- 7 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs index 79388a991ea..a38f6d2a8a2 100644 --- a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs +++ b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs @@ -1302,6 +1302,11 @@ private void GenerateSplitTableMapping( using (stringBuilder.Indent()) { + if (fragment.IsOptional) + { + stringBuilder.AppendLine().Append("t.IsOptional();"); + } + GenerateTriggers("t", entityType, table.Name, table.Schema, stringBuilder); GeneratePropertyOverrides("t", entityType, table, stringBuilder); GenerateEntityTypeMappingFragmentAnnotations("t", fragment, stringBuilder); diff --git a/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs b/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs index 7ec22ca8cb1..4d2acc25722 100644 --- a/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs +++ b/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs @@ -1789,7 +1789,14 @@ private void GenerateAddMapping( if (tableMapping.IsSplitEntityTypePrincipal.HasValue) { mainBuilder - .Append("IsSplitEntityTypePrincipal = ").AppendLine(code.Literal(tableMapping.IsSplitEntityTypePrincipal)); + .Append("IsSplitEntityTypePrincipal = ").Append(code.Literal(tableMapping.IsSplitEntityTypePrincipal)); + mainBuilder.AppendLine(tableMapping.IsSplitFragmentOptional ? "," : ""); + } + + if (tableMapping.IsSplitFragmentOptional) + { + mainBuilder + .Append("IsSplitFragmentOptional = ").AppendLine(code.Literal(tableMapping.IsSplitFragmentOptional)); } mainBuilder.DecrementIndent().AppendLine("};"); @@ -2184,7 +2191,8 @@ private void Create( AppendLiteral(storeObject, mainBuilder, code); mainBuilder.AppendLine(",") - .Append(code.Literal(fragment.IsTableExcludedFromMigrations)).AppendLine(");").DecrementIndent(); + .Append(code.Literal(fragment.IsTableExcludedFromMigrations)).AppendLine(",") + .Append(code.Literal(fragment.IsOptional)).AppendLine(");").DecrementIndent(); CreateAnnotations( fragment, diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs index 45b8af21534..eff82a712af 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs @@ -251,7 +251,8 @@ private static RuntimeEntityTypeMappingFragment Create( => new( runtimeEntityType, entityTypeMappingFragment.StoreObject, - entityTypeMappingFragment.IsTableExcludedFromMigrations); + entityTypeMappingFragment.IsTableExcludedFromMigrations, + entityTypeMappingFragment.IsOptional); /// /// Updates the relational property overrides annotations that will be set on the read-only object. diff --git a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/OwnedTypeEntityType.cs b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/OwnedTypeEntityType.cs index a2838aeefb9..d900a0a9e99 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/OwnedTypeEntityType.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/OwnedTypeEntityType.cs @@ -749,7 +749,8 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) var detailsFragment = new RuntimeEntityTypeMappingFragment( runtimeEntityType, StoreObjectIdentifier.Table("Details", null), - null); + null, + false); fragments.Add(StoreObjectIdentifier.Table("Details", null), detailsFragment); runtimeEntityType.AddAnnotation("Relational:MappingFragments", fragments); runtimeEntityType.AddAnnotation("Relational:FunctionName", null); diff --git a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/OwnedTypeEntityType.cs b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/OwnedTypeEntityType.cs index ad6ffc6c9d9..f10ea1f353e 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/OwnedTypeEntityType.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/OwnedTypeEntityType.cs @@ -219,7 +219,8 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) var detailsFragment = new RuntimeEntityTypeMappingFragment( runtimeEntityType, StoreObjectIdentifier.Table("Details", null), - null); + null, + false); fragments.Add(StoreObjectIdentifier.Table("Details", null), detailsFragment); runtimeEntityType.AddAnnotation("Relational:MappingFragments", fragments); runtimeEntityType.AddAnnotation("Relational:FunctionName", null); diff --git a/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/OwnedTypeEntityType.cs b/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/OwnedTypeEntityType.cs index 77d68a4682d..d788732fd41 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/OwnedTypeEntityType.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/OwnedTypeEntityType.cs @@ -683,7 +683,8 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) var detailsFragment = new RuntimeEntityTypeMappingFragment( runtimeEntityType, StoreObjectIdentifier.Table("Details", null), - null); + null, + false); fragments.Add(StoreObjectIdentifier.Table("Details", null), detailsFragment); runtimeEntityType.AddAnnotation("Relational:MappingFragments", fragments); runtimeEntityType.AddAnnotation("Relational:FunctionName", null); diff --git a/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/OwnedTypeEntityType.cs b/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/OwnedTypeEntityType.cs index 9240baeec74..7d026ba5f16 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/OwnedTypeEntityType.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/OwnedTypeEntityType.cs @@ -205,7 +205,8 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) var detailsFragment = new RuntimeEntityTypeMappingFragment( runtimeEntityType, StoreObjectIdentifier.Table("Details", null), - null); + null, + false); fragments.Add(StoreObjectIdentifier.Table("Details", null), detailsFragment); runtimeEntityType.AddAnnotation("Relational:MappingFragments", fragments); runtimeEntityType.AddAnnotation("Relational:FunctionName", null); From cd26e9369bba9e0aea88bb575735d761644dcac1 Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Fri, 31 Jul 2026 17:41:10 +0500 Subject: [PATCH 07/11] Update API baseline for optional entity-splitting fragments Part of #27974 --- .../EFCore.Relational.baseline.json | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/src/EFCore.Relational/EFCore.Relational.baseline.json b/src/EFCore.Relational/EFCore.Relational.baseline.json index 86e54eddedc..d470bf02dfe 100644 --- a/src/EFCore.Relational/EFCore.Relational.baseline.json +++ b/src/EFCore.Relational/EFCore.Relational.baseline.json @@ -3895,9 +3895,15 @@ { "Member": "Microsoft.EntityFrameworkCore.Metadata.ConfigurationSource GetConfigurationSource();" }, + { + "Member": "Microsoft.EntityFrameworkCore.Metadata.ConfigurationSource? GetIsOptionalConfigurationSource();" + }, { "Member": "Microsoft.EntityFrameworkCore.Metadata.ConfigurationSource? GetIsTableExcludedFromMigrationsConfigurationSource();" }, + { + "Member": "bool? SetIsOptional(bool? optional, bool fromDataAnnotation = false);" + }, { "Member": "bool? SetIsTableExcludedFromMigrations(bool? excluded, bool fromDataAnnotation = false);" } @@ -5010,6 +5016,9 @@ { "Member": "Microsoft.EntityFrameworkCore.Metadata.IMutableEntityType EntityType { get; }" }, + { + "Member": "bool IsOptional { get; set; }" + }, { "Member": "bool? IsTableExcludedFromMigrations { get; set; }" } @@ -5588,6 +5597,9 @@ { "Member": "Microsoft.EntityFrameworkCore.Metadata.IReadOnlyEntityType EntityType { get; }" }, + { + "Member": "bool IsOptional { get; }" + }, { "Member": "bool? IsTableExcludedFromMigrations { get; }" }, @@ -7171,6 +7183,9 @@ { "Member": "bool? IsSplitEntityTypePrincipal { get; }" }, + { + "Member": "bool IsSplitFragmentOptional { get; }" + }, { "Member": "Microsoft.EntityFrameworkCore.Metadata.ITableBase Table { get; }" }, @@ -7239,6 +7254,12 @@ { "Member": "void AppendObtainNextSequenceValueOperation(System.Text.StringBuilder commandStringBuilder, string name, string? schema);" }, + { + "Member": "Microsoft.EntityFrameworkCore.Update.ResultSetMapping AppendOptionalFragmentDeleteOperation(System.Text.StringBuilder commandStringBuilder, Microsoft.EntityFrameworkCore.Update.IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction);" + }, + { + "Member": "Microsoft.EntityFrameworkCore.Update.ResultSetMapping AppendOptionalFragmentUpsertOperation(System.Text.StringBuilder commandStringBuilder, Microsoft.EntityFrameworkCore.Update.IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction);" + }, { "Member": "Microsoft.EntityFrameworkCore.Update.ResultSetMapping AppendStoredProcedureCall(System.Text.StringBuilder commandStringBuilder, Microsoft.EntityFrameworkCore.Update.IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction);" }, @@ -8506,6 +8527,9 @@ { "Member": "virtual System.Collections.Generic.IReadOnlyList Entries { get; }" }, + { + "Member": "virtual bool IsOptionalSplitFragment { get; }" + }, { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.IColumnBase? RowsAffectedColumn { get; private set; }" }, @@ -8816,6 +8840,9 @@ { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.TableTriggerBuilder HasTrigger(string modelName);" }, + { + "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.OwnedNavigationSplitTableBuilder IsOptional(bool optional = true);" + }, { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.ColumnBuilder Property(string propertyName);" }, @@ -8850,6 +8877,9 @@ { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.OwnedNavigationSplitTableBuilder HasAnnotation(string annotation, object? value);" }, + { + "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.OwnedNavigationSplitTableBuilder IsOptional(bool optional = true);" + }, { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.ColumnBuilder Property(System.Linq.Expressions.Expression> propertyExpression);" } @@ -9988,6 +10018,10 @@ "Member": "const string DeleteStoredProcedureParameterMappings", "Value": "Relational:DeleteStoredProcedureParameterMappings" }, + { + "Member": "const string EntityTypeMappingFragmentIsOptional", + "Value": "Relational:EntityTypeMappingFragmentIsOptional" + }, { "Member": "const string FieldValueGetter", "Value": "Relational:FieldValueGetter" @@ -16501,6 +16535,12 @@ { "Member": "static string EntitySplittingMissingRequiredPropertiesOptionalDependent(object? entityType, object? storeObject, object? requiredDependentConfig);" }, + { + "Member": "static string EntitySplittingNonNullablePropertyOnOptionalFragment(object? entityType, object? storeObject, object? property);" + }, + { + "Member": "static string EntitySplittingOptionalFragmentSharedTable(object? entityType, object? storeObject, object? principalEntityType);" + }, { "Member": "static string EntitySplittingUnmappedMainFragment(object? entityType, object? storeObject, object? storeObjectType);" }, @@ -17182,6 +17222,9 @@ { "Member": "static string OnlyConstantsSupportedInInlineCollectionQueryRoots { get; }" }, + { + "Member": "static string OptionalEntitySplittingNotSupported { get; }" + }, { "Member": "static string PendingAmbientTransaction { get; }" }, @@ -18347,7 +18390,7 @@ "Type": "class Microsoft.EntityFrameworkCore.Metadata.RuntimeEntityTypeMappingFragment : Microsoft.EntityFrameworkCore.Infrastructure.AnnotatableBase, Microsoft.EntityFrameworkCore.Metadata.IEntityTypeMappingFragment, Microsoft.EntityFrameworkCore.Metadata.IReadOnlyEntityTypeMappingFragment, Microsoft.EntityFrameworkCore.Infrastructure.IReadOnlyAnnotatable, Microsoft.EntityFrameworkCore.Infrastructure.IAnnotatable", "Methods": [ { - "Member": "RuntimeEntityTypeMappingFragment(Microsoft.EntityFrameworkCore.Metadata.RuntimeEntityType entityType, in Microsoft.EntityFrameworkCore.Metadata.StoreObjectIdentifier storeObject, bool? isTableExcludedFromMigrations);" + "Member": "RuntimeEntityTypeMappingFragment(Microsoft.EntityFrameworkCore.Metadata.RuntimeEntityType entityType, in Microsoft.EntityFrameworkCore.Metadata.StoreObjectIdentifier storeObject, bool? isTableExcludedFromMigrations, bool isOptional = false);" }, { "Member": "override string ToString();" @@ -18357,6 +18400,9 @@ { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.RuntimeEntityType EntityType { get; }" }, + { + "Member": "virtual bool IsOptional { get; }" + }, { "Member": "virtual bool? IsTableExcludedFromMigrations { get; }" }, @@ -18910,6 +18956,9 @@ { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.TableTriggerBuilder HasTrigger(string modelName);" }, + { + "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.SplitTableBuilder IsOptional(bool optional = true);" + }, { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.ColumnBuilder Property(string propertyName);" }, @@ -18944,6 +18993,9 @@ { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.SplitTableBuilder HasAnnotation(string annotation, object? value);" }, + { + "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.SplitTableBuilder IsOptional(bool optional = true);" + }, { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.Builders.ColumnBuilder Property(System.Linq.Expressions.Expression> propertyExpression);" } @@ -20964,6 +21016,12 @@ { "Member": "virtual void AppendObtainNextSequenceValueOperation(System.Text.StringBuilder commandStringBuilder, string name, string? schema);" }, + { + "Member": "virtual Microsoft.EntityFrameworkCore.Update.ResultSetMapping AppendOptionalFragmentDeleteOperation(System.Text.StringBuilder commandStringBuilder, Microsoft.EntityFrameworkCore.Update.IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction);" + }, + { + "Member": "virtual Microsoft.EntityFrameworkCore.Update.ResultSetMapping AppendOptionalFragmentUpsertOperation(System.Text.StringBuilder commandStringBuilder, Microsoft.EntityFrameworkCore.Update.IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction);" + }, { "Member": "virtual void AppendReturningClause(System.Text.StringBuilder commandStringBuilder, System.Collections.Generic.IReadOnlyList operations, string? additionalValues = null);" }, From e68cbccc6907813af503e931ce5708e78efe1f74 Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Sat, 1 Aug 2026 13:12:56 +0500 Subject: [PATCH 08/11] Decide insert/update/delete for optional entity-splitting fragments from tracked values Part of #27974 --- .../EFCore.Relational.baseline.json | 18 +- .../Properties/RelationalStrings.Designer.cs | 6 - .../Properties/RelationalStrings.resx | 3 - .../Update/IUpdateSqlGenerator.cs | 32 --- .../Update/Internal/CommandBatchPreparer.cs | 6 + .../Update/ModificationCommand.cs | 21 ++ .../Update/ReaderModificationCommandBatch.cs | 16 +- .../Update/UpdateSqlGenerator.cs | 16 -- .../Internal/SqlServerUpdateSqlGenerator.cs | 212 ----------------- .../Update/SqlServerUpdateSqlGeneratorTest.cs | 216 ------------------ .../OptionalEntitySplittingSqliteTest.cs | 139 +++++++++-- 11 files changed, 151 insertions(+), 534 deletions(-) diff --git a/src/EFCore.Relational/EFCore.Relational.baseline.json b/src/EFCore.Relational/EFCore.Relational.baseline.json index d470bf02dfe..ad6f0467b72 100644 --- a/src/EFCore.Relational/EFCore.Relational.baseline.json +++ b/src/EFCore.Relational/EFCore.Relational.baseline.json @@ -7254,12 +7254,6 @@ { "Member": "void AppendObtainNextSequenceValueOperation(System.Text.StringBuilder commandStringBuilder, string name, string? schema);" }, - { - "Member": "Microsoft.EntityFrameworkCore.Update.ResultSetMapping AppendOptionalFragmentDeleteOperation(System.Text.StringBuilder commandStringBuilder, Microsoft.EntityFrameworkCore.Update.IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction);" - }, - { - "Member": "Microsoft.EntityFrameworkCore.Update.ResultSetMapping AppendOptionalFragmentUpsertOperation(System.Text.StringBuilder commandStringBuilder, Microsoft.EntityFrameworkCore.Update.IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction);" - }, { "Member": "Microsoft.EntityFrameworkCore.Update.ResultSetMapping AppendStoredProcedureCall(System.Text.StringBuilder commandStringBuilder, Microsoft.EntityFrameworkCore.Update.IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction);" }, @@ -8530,6 +8524,9 @@ { "Member": "virtual bool IsOptionalSplitFragment { get; }" }, + { + "Member": "virtual bool IsOptionalSplitFragmentRowAssumedAbsent { get; }" + }, { "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.IColumnBase? RowsAffectedColumn { get; private set; }" }, @@ -17222,9 +17219,6 @@ { "Member": "static string OnlyConstantsSupportedInInlineCollectionQueryRoots { get; }" }, - { - "Member": "static string OptionalEntitySplittingNotSupported { get; }" - }, { "Member": "static string PendingAmbientTransaction { get; }" }, @@ -21016,12 +21010,6 @@ { "Member": "virtual void AppendObtainNextSequenceValueOperation(System.Text.StringBuilder commandStringBuilder, string name, string? schema);" }, - { - "Member": "virtual Microsoft.EntityFrameworkCore.Update.ResultSetMapping AppendOptionalFragmentDeleteOperation(System.Text.StringBuilder commandStringBuilder, Microsoft.EntityFrameworkCore.Update.IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction);" - }, - { - "Member": "virtual Microsoft.EntityFrameworkCore.Update.ResultSetMapping AppendOptionalFragmentUpsertOperation(System.Text.StringBuilder commandStringBuilder, Microsoft.EntityFrameworkCore.Update.IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction);" - }, { "Member": "virtual void AppendReturningClause(System.Text.StringBuilder commandStringBuilder, System.Collections.Generic.IReadOnlyList operations, string? additionalValues = null);" }, diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index dd510b1e2d9..29f755dac4f 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -1805,12 +1805,6 @@ public static string OptionalDependentWithDependentWithoutIdentifyingProperty(ob GetString("OptionalDependentWithDependentWithoutIdentifyingProperty", nameof(entityType)), entityType); - /// - /// Saving changes to an optional entity-splitting fragment is not supported by the current database provider. Either configure the fragment as required by removing the call to 'IsOptional()', or use a provider that supports optional entity-splitting fragments for 'SaveChanges'. - /// - public static string OptionalEntitySplittingNotSupported - => GetString("OptionalEntitySplittingNotSupported"); - /// /// The value provided for parameter '{parameter}' cannot be used because it isn't assignable to type 'object[]'. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index ea0578deee7..31c67f9be35 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1146,9 +1146,6 @@ Entity type '{entityType}' is an optional dependent using table sharing and containing other dependents without any required non shared property to identify whether the entity exists. If all nullable properties contain a 'null' value in database then an object instance won't be created in the query causing nested dependent's values to be lost. Add a required property to create instances with 'null' values for other properties or mark the incoming navigation as required to always create an instance. - - Saving changes to an optional entity-splitting fragment is not supported by the current database provider. Either configure the fragment as required by removing the call to 'IsOptional()', or use a provider that supports optional entity-splitting fragments for 'SaveChanges'. - The value provided for parameter '{parameter}' cannot be used because it isn't assignable to type 'object[]'. diff --git a/src/EFCore.Relational/Update/IUpdateSqlGenerator.cs b/src/EFCore.Relational/Update/IUpdateSqlGenerator.cs index 5a7842162d8..aca1270f895 100644 --- a/src/EFCore.Relational/Update/IUpdateSqlGenerator.cs +++ b/src/EFCore.Relational/Update/IUpdateSqlGenerator.cs @@ -174,36 +174,4 @@ ResultSetMapping AppendStoredProcedureCall( IReadOnlyModificationCommand command, int commandPosition, out bool requiresTransaction); - - /// - /// Appends SQL that atomically updates the row for an optional entity-splitting fragment if it exists, inserts it if it - /// doesn't and at least one non-key value being written is non-, or leaves the row absent if it - /// doesn't exist and every non-key value being written is . - /// - /// The builder to which the SQL should be appended. - /// The command that represents the update operation. - /// The ordinal of this command in the batch. - /// Returns whether the SQL appended must be executed in a transaction to work correctly. - /// The for the command. - ResultSetMapping AppendOptionalFragmentUpsertOperation( - StringBuilder commandStringBuilder, - IReadOnlyModificationCommand command, - int commandPosition, - out bool requiresTransaction) - => throw new NotSupportedException(RelationalStrings.OptionalEntitySplittingNotSupported); - - /// - /// Appends SQL that deletes the row for an optional entity-splitting fragment, tolerating the row already being absent. - /// - /// The builder to which the SQL should be appended. - /// The command that represents the delete operation. - /// The ordinal of this command in the batch. - /// Returns whether the SQL appended must be executed in a transaction to work correctly. - /// The for the command. - ResultSetMapping AppendOptionalFragmentDeleteOperation( - StringBuilder commandStringBuilder, - IReadOnlyModificationCommand command, - int commandPosition, - out bool requiresTransaction) - => throw new NotSupportedException(RelationalStrings.OptionalEntitySplittingNotSupported); } diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index a86d8580484..29dd0f93425 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -130,6 +130,12 @@ private IEnumerable CreateCommandBatches( continue; } + if (modificationCommand.EntityState == EntityState.Deleted + && modificationCommand is ModificationCommand { IsOptionalSplitFragmentRowAssumedAbsent: true }) + { + continue; + } + if (!batch.TryAddCommand(modificationCommand)) { if (batch.ModificationCommands.Count == 1 diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs index 301712a20ad..56f594bf1eb 100644 --- a/src/EFCore.Relational/Update/ModificationCommand.cs +++ b/src/EFCore.Relational/Update/ModificationCommand.cs @@ -103,6 +103,19 @@ public virtual bool IsOptionalSplitFragment && _entries.Count > 0 && GetTableMapping(_entries[0].EntityType) is { IsSplitFragmentOptional: true }; + /// + /// Gets a value indicating whether this command targets an optional entity-splitting fragment table whose row is + /// assumed not to exist, because every non-key property mapped to it had a original value + /// when the entity was loaded or last saved. + /// + public virtual bool IsOptionalSplitFragmentRowAssumedAbsent + => StoreStoredProcedure is null + && _entries.Count > 0 + && GetTableMapping(_entries[0].EntityType) is { IsSplitFragmentOptional: true } tableMapping + && tableMapping.ColumnMappings + .Where(m => !m.Property.IsPrimaryKey()) + .All(m => _entries[0].GetOriginalValue(m.Property) is null); + /// /// The list of needed to perform the insert, update, or delete. /// @@ -172,6 +185,14 @@ public virtual void AddEntry(IUpdateEntry entry, bool mainEntry) .Any(m => m.Table.Name == TableName && m.Table.Schema == Schema) ? EntityState.Modified : entry.EntityState; + + // An optional entity-splitting fragment's row may not exist yet. If every non-key value mapped to it was + // null when the entity was loaded, assume the row is absent and insert it rather than updating it. + if (_entityState == EntityState.Modified + && IsOptionalSplitFragmentRowAssumedAbsent) + { + _entityState = EntityState.Added; + } } else { diff --git a/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs b/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs index 5f7f0a98d19..b85778783ac 100644 --- a/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs +++ b/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs @@ -204,8 +204,6 @@ protected virtual void AddCommand(IReadOnlyModificationCommand modificationComma } else { - var isOptionalSplitFragment = modificationCommand is ModificationCommand { IsOptionalSplitFragment: true }; - switch (modificationCommand.EntityState) { case EntityState.Added: @@ -215,19 +213,13 @@ protected virtual void AddCommand(IReadOnlyModificationCommand modificationComma break; case EntityState.Modified: ResultSetMappings.Add( - isOptionalSplitFragment - ? UpdateSqlGenerator.AppendOptionalFragmentUpsertOperation( - SqlBuilder, modificationCommand, commandPosition, out requiresTransaction) - : UpdateSqlGenerator.AppendUpdateOperation( - SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); + UpdateSqlGenerator.AppendUpdateOperation( + SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); break; case EntityState.Deleted: ResultSetMappings.Add( - isOptionalSplitFragment - ? UpdateSqlGenerator.AppendOptionalFragmentDeleteOperation( - SqlBuilder, modificationCommand, commandPosition, out requiresTransaction) - : UpdateSqlGenerator.AppendDeleteOperation( - SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); + UpdateSqlGenerator.AppendDeleteOperation( + SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); break; default: diff --git a/src/EFCore.Relational/Update/UpdateSqlGenerator.cs b/src/EFCore.Relational/Update/UpdateSqlGenerator.cs index 3335031fe7c..a1b5ac922da 100644 --- a/src/EFCore.Relational/Update/UpdateSqlGenerator.cs +++ b/src/EFCore.Relational/Update/UpdateSqlGenerator.cs @@ -433,22 +433,6 @@ public virtual ResultSetMapping AppendStoredProcedureCall( return resultSetMapping; } - /// - public virtual ResultSetMapping AppendOptionalFragmentUpsertOperation( - StringBuilder commandStringBuilder, - IReadOnlyModificationCommand command, - int commandPosition, - out bool requiresTransaction) - => throw new NotSupportedException(RelationalStrings.OptionalEntitySplittingNotSupported); - - /// - public virtual ResultSetMapping AppendOptionalFragmentDeleteOperation( - StringBuilder commandStringBuilder, - IReadOnlyModificationCommand command, - int commandPosition, - out bool requiresTransaction) - => throw new NotSupportedException(RelationalStrings.OptionalEntitySplittingNotSupported); - /// /// Appends a SQL fragment for a VALUES. /// diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs index cb72c0de12d..1999f30bd98 100644 --- a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs +++ b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs @@ -217,218 +217,6 @@ protected override void AppendDeleteCommand( commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); } - /// - /// 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. - /// - /// - /// Generates an UPDATE and, only when it affects no rows and at least one non-key value being written is - /// non-, a follow-up INSERT: the row for an optional entity-splitting fragment is - /// created lazily and is atomically updated or inserted within a single round trip, without using MERGE. - /// A row-count mismatch after the UPDATE that isn't explained by the row simply being absent is treated as - /// a genuine concurrency conflict and reported through the normal affected-count consumption pipeline. - /// - public override ResultSetMapping AppendOptionalFragmentUpsertOperation( - StringBuilder commandStringBuilder, - IReadOnlyModificationCommand command, - int commandPosition, - out bool requiresTransaction) - { - var name = command.TableName; - var schema = command.Schema; - var operations = command.ColumnModifications; - - var writeOperations = operations.Where(o => o.IsWrite).ToList(); - var keyOperations = operations.Where(o => o.IsKey).ToList(); - var conditionOperations = operations.Where(o => o.IsCondition).ToList(); - var tokenOperations = conditionOperations.Where(o => !o.IsKey).ToList(); - var readOperations = operations.Where(o => o.IsRead).ToList(); - - if (readOperations.Count > 0) - { - throw new NotSupportedException(RelationalStrings.OptionalEntitySplittingNotSupported); - } - - requiresTransaction = true; - - var rowsAffectedVariable = "@_fragmentRowsAffected" + commandPosition.ToString(CultureInfo.InvariantCulture); - - commandStringBuilder - .Append("DECLARE ").Append(rowsAffectedVariable).Append(" int") - .AppendLine(SqlGenerationHelper.StatementTerminator) - .AppendLine(); - - AppendUpdateCommandHeader(commandStringBuilder, name, schema, writeOperations); - AppendWhereClause(commandStringBuilder, conditionOperations); - commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); - - commandStringBuilder - .Append("SET ").Append(rowsAffectedVariable).Append(" = @@ROWCOUNT") - .AppendLine(SqlGenerationHelper.StatementTerminator) - .AppendLine(); - - commandStringBuilder - .Append("IF ").Append(rowsAffectedVariable).AppendLine(" = 0") - .AppendLine("BEGIN"); - - var indent = " "; - if (tokenOperations.Count > 0) - { - // A non-key (concurrency token) condition narrowed the UPDATE. Zero rows affected could mean either that the - // row doesn't exist (fine, no conflict) or that it exists but the token didn't match (a genuine conflict). - commandStringBuilder.Append(indent).Append("IF NOT EXISTS (SELECT 1 FROM "); - SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, name, schema); - commandStringBuilder - .Append(" WHERE ") - .AppendJoin(keyOperations, (sb, o) => AppendWhereCondition(sb, o, useOriginalValue: true), " AND ") - .AppendLine(")") - .Append(indent).AppendLine("BEGIN"); - indent += " "; - } - - commandStringBuilder.Append(indent).Append("IF (") - .AppendJoin( - writeOperations, (sb, o) => - { - AppendParameterOrLiteral(sb, o, useOriginal: false); - sb.Append(" IS NOT NULL"); - }, " OR ") - .AppendLine(")") - .Append(indent).AppendLine("BEGIN"); - - var insertColumns = keyOperations.Concat(writeOperations).ToList(); - commandStringBuilder.Append(indent).Append(" INSERT INTO "); - SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, name, schema); - commandStringBuilder - .Append(" (") - .AppendJoin(insertColumns, SqlGenerationHelper, (sb, o, helper) => helper.DelimitIdentifier(sb, o.ColumnName)) - .AppendLine(")") - .Append(indent).Append(" VALUES (") - .AppendJoin( - keyOperations, (sb, o) => AppendParameterOrLiteral(sb, o, useOriginal: true), ", ") - .Append(keyOperations.Count > 0 && writeOperations.Count > 0 ? ", " : "") - .AppendJoin(writeOperations, (sb, o) => AppendParameterOrLiteral(sb, o, useOriginal: false), ", ") - .Append(")") - .AppendLine(SqlGenerationHelper.StatementTerminator) - .Append(indent).Append(" SET ").Append(rowsAffectedVariable).Append(" = @@ROWCOUNT") - .AppendLine(SqlGenerationHelper.StatementTerminator) - .Append(indent).AppendLine("END") - .Append(indent).AppendLine("ELSE") - .Append(indent).AppendLine("BEGIN") - .Append(indent).Append(" SET ").Append(rowsAffectedVariable).Append(" = 1") - .AppendLine(SqlGenerationHelper.StatementTerminator) - .Append(indent).AppendLine("END"); - - if (tokenOperations.Count > 0) - { - commandStringBuilder.AppendLine(" END"); - } - - commandStringBuilder.AppendLine("END").AppendLine(); - - commandStringBuilder - .Append("SELECT ").Append(rowsAffectedVariable) - .AppendLine(SqlGenerationHelper.StatementTerminator) - .AppendLine(); - - return ResultSetMapping.LastInResultSet | ResultSetMapping.ResultSetWithRowsAffectedOnly; - } - - /// - /// 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. - /// - /// - /// Generates a DELETE for an optional entity-splitting fragment that tolerates the row already being - /// absent: zero rows affected is only reported as a genuine concurrency conflict when a concurrency token - /// condition narrowed the DELETE and a row with the given key still exists. - /// - public override ResultSetMapping AppendOptionalFragmentDeleteOperation( - StringBuilder commandStringBuilder, - IReadOnlyModificationCommand command, - int commandPosition, - out bool requiresTransaction) - { - var name = command.TableName; - var schema = command.Schema; - var operations = command.ColumnModifications; - - var keyOperations = operations.Where(o => o.IsKey).ToList(); - var conditionOperations = operations.Where(o => o.IsCondition).ToList(); - var tokenOperations = conditionOperations.Where(o => !o.IsKey).ToList(); - - requiresTransaction = true; - - var rowsAffectedVariable = "@_fragmentRowsAffected" + commandPosition.ToString(CultureInfo.InvariantCulture); - - commandStringBuilder - .Append("DECLARE ").Append(rowsAffectedVariable).Append(" int") - .AppendLine(SqlGenerationHelper.StatementTerminator) - .AppendLine(); - - commandStringBuilder.Append("DELETE FROM "); - SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, name, schema); - AppendWhereClause(commandStringBuilder, conditionOperations); - commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator); - - commandStringBuilder - .Append("SET ").Append(rowsAffectedVariable).Append(" = @@ROWCOUNT") - .AppendLine(SqlGenerationHelper.StatementTerminator) - .AppendLine(); - - commandStringBuilder - .Append("IF ").Append(rowsAffectedVariable).AppendLine(" = 0") - .AppendLine("BEGIN"); - - if (tokenOperations.Count > 0) - { - // The DELETE's concurrency-token condition could be why no rows were affected. Treat the row as tolerably - // absent only if a row with the key genuinely doesn't exist; otherwise this is a real concurrency conflict. - commandStringBuilder.Append(" IF NOT EXISTS (SELECT 1 FROM "); - SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, name, schema); - commandStringBuilder - .Append(" WHERE ") - .AppendJoin(keyOperations, (sb, o) => AppendWhereCondition(sb, o, useOriginalValue: true), " AND ") - .AppendLine(")") - .AppendLine(" BEGIN") - .Append(" SET ").Append(rowsAffectedVariable).Append(" = 1") - .AppendLine(SqlGenerationHelper.StatementTerminator) - .AppendLine(" END"); - } - else - { - commandStringBuilder - .Append(" SET ").Append(rowsAffectedVariable).Append(" = 1") - .AppendLine(SqlGenerationHelper.StatementTerminator); - } - - commandStringBuilder.AppendLine("END").AppendLine(); - - commandStringBuilder - .Append("SELECT ").Append(rowsAffectedVariable) - .AppendLine(SqlGenerationHelper.StatementTerminator) - .AppendLine(); - - return ResultSetMapping.LastInResultSet | ResultSetMapping.ResultSetWithRowsAffectedOnly; - } - - private void AppendParameterOrLiteral(StringBuilder commandStringBuilder, IColumnModification columnModification, bool useOriginal) - { - if (useOriginal ? columnModification.UseOriginalValueParameter : columnModification.UseCurrentValueParameter) - { - SqlGenerationHelper.GenerateParameterNamePlaceholder( - commandStringBuilder, useOriginal ? columnModification.OriginalParameterName! : columnModification.ParameterName!); - } - else - { - AppendSqlLiteral(commandStringBuilder, columnModification, null, null); - } - } - /// /// 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 diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs index dc1bae35304..23663830a89 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs @@ -245,220 +245,4 @@ protected override string CloseDelimiter private void AssertBaseline(string expected, string actual) => Assert.Equal(expected, actual.TrimEnd(), ignoreLineEndingDifferences: true); - - [Fact] - public void AppendOptionalFragmentUpsertOperation_generates_update_then_conditional_insert() - { - var stringBuilder = new StringBuilder(); - var command = CreateOptionalFragmentCommand(EntityState.Modified, concurrencyToken: false); - - CreateSqlGenerator().AppendOptionalFragmentUpsertOperation(stringBuilder, command, 0, out var requiresTransaction); - - AssertBaseline( - """ -DECLARE @_fragmentRowsAffected0 int; - -UPDATE [dbo].[CustomerDetails] SET [Description] = @p0, [Score] = @p1 -WHERE [Id] = @p2; -SET @_fragmentRowsAffected0 = @@ROWCOUNT; - -IF @_fragmentRowsAffected0 = 0 -BEGIN - IF (@p0 IS NOT NULL OR @p1 IS NOT NULL) - BEGIN - INSERT INTO [dbo].[CustomerDetails] ([Id], [Description], [Score]) - VALUES (@p2, @p0, @p1); - SET @_fragmentRowsAffected0 = @@ROWCOUNT; - END - ELSE - BEGIN - SET @_fragmentRowsAffected0 = 1; - END -END - -SELECT @_fragmentRowsAffected0; -""", - stringBuilder.ToString()); - Assert.True(requiresTransaction); - } - - [Fact] - public void AppendOptionalFragmentUpsertOperation_distinguishes_absence_from_conflict_with_concurrency_token() - { - var stringBuilder = new StringBuilder(); - var command = CreateOptionalFragmentCommand(EntityState.Modified, concurrencyToken: true); - - CreateSqlGenerator().AppendOptionalFragmentUpsertOperation(stringBuilder, command, 0, out var requiresTransaction); - - AssertBaseline( - """ -DECLARE @_fragmentRowsAffected0 int; - -UPDATE [dbo].[CustomerDetails] SET [Description] = @p0, [Score] = @p1 -WHERE [Id] = @p2 AND [Version] IS NULL; -SET @_fragmentRowsAffected0 = @@ROWCOUNT; - -IF @_fragmentRowsAffected0 = 0 -BEGIN - IF NOT EXISTS (SELECT 1 FROM [dbo].[CustomerDetails] WHERE [Id] = @p2) - BEGIN - IF (@p0 IS NOT NULL OR @p1 IS NOT NULL) - BEGIN - INSERT INTO [dbo].[CustomerDetails] ([Id], [Description], [Score]) - VALUES (@p2, @p0, @p1); - SET @_fragmentRowsAffected0 = @@ROWCOUNT; - END - ELSE - BEGIN - SET @_fragmentRowsAffected0 = 1; - END - END -END - -SELECT @_fragmentRowsAffected0; -""", - stringBuilder.ToString()); - Assert.True(requiresTransaction); - } - - [Fact] - public void AppendOptionalFragmentDeleteOperation_generates_tolerant_delete() - { - var stringBuilder = new StringBuilder(); - var command = CreateOptionalFragmentCommand(EntityState.Deleted, concurrencyToken: false); - - CreateSqlGenerator().AppendOptionalFragmentDeleteOperation(stringBuilder, command, 0, out var requiresTransaction); - - AssertBaseline( - """ -DECLARE @_fragmentRowsAffected0 int; - -DELETE FROM [dbo].[CustomerDetails] -WHERE [Id] = @p0; -SET @_fragmentRowsAffected0 = @@ROWCOUNT; - -IF @_fragmentRowsAffected0 = 0 -BEGIN - SET @_fragmentRowsAffected0 = 1; -END - -SELECT @_fragmentRowsAffected0; -""", - stringBuilder.ToString()); - Assert.True(requiresTransaction); - } - - [Fact] - public void AppendOptionalFragmentDeleteOperation_distinguishes_absence_from_conflict_with_concurrency_token() - { - var stringBuilder = new StringBuilder(); - var command = CreateOptionalFragmentCommand(EntityState.Deleted, concurrencyToken: true); - - CreateSqlGenerator().AppendOptionalFragmentDeleteOperation(stringBuilder, command, 0, out var requiresTransaction); - - AssertBaseline( - """ -DECLARE @_fragmentRowsAffected0 int; - -DELETE FROM [dbo].[CustomerDetails] -WHERE [Id] = @p0 AND [Version] IS NULL; -SET @_fragmentRowsAffected0 = @@ROWCOUNT; - -IF @_fragmentRowsAffected0 = 0 -BEGIN - IF NOT EXISTS (SELECT 1 FROM [dbo].[CustomerDetails] WHERE [Id] = @p0) - BEGIN - SET @_fragmentRowsAffected0 = 1; - END -END - -SELECT @_fragmentRowsAffected0; -""", - stringBuilder.ToString()); - Assert.True(requiresTransaction); - } - - [Fact] - public void AppendOptionalFragmentUpsertOperation_throws_for_generated_values() - { - var stringBuilder = new StringBuilder(); - var command = CreateOptionalFragmentCommand(EntityState.Modified, concurrencyToken: false, generatedValue: true); - - Assert.Throws( - () => CreateSqlGenerator().AppendOptionalFragmentUpsertOperation(stringBuilder, command, 0, out _)); - } - - private IModificationCommand CreateOptionalFragmentCommand( - EntityState entityState, - bool concurrencyToken, - bool generatedValue = false) - { - var model = GetOptionalFragmentModel(); - var stateManager = TestHelpers.CreateContextServices(model).GetRequiredService(); - var entry = stateManager.GetOrCreateEntry(new OptionalFragmentDetail()); - entry.SetEntityState(entityState); - var generator = new ParameterNameGenerator(); - - var detailType = entry.EntityType; - var idProperty = detailType.FindProperty(nameof(OptionalFragmentDetail.Id)); - var descriptionProperty = detailType.FindProperty(nameof(OptionalFragmentDetail.Description)); - var scoreProperty = detailType.FindProperty(nameof(OptionalFragmentDetail.Score)); - var versionProperty = detailType.FindProperty(nameof(OptionalFragmentDetail.Version)); - - var columnModifications = new List - { - new( - entry, idProperty, idProperty.GetTableColumnMappings().Single().Column, generator.GenerateNext, - idProperty.GetTableColumnMappings().Single().TypeMapping, false, false, true, true, true) - }; - - if (entityState != EntityState.Deleted) - { - columnModifications.Add( - new( - entry, descriptionProperty, descriptionProperty.GetTableColumnMappings().Single().Column, generator.GenerateNext, - descriptionProperty.GetTableColumnMappings().Single().TypeMapping, false, true, false, false, true)); - columnModifications.Add( - new( - entry, scoreProperty, scoreProperty.GetTableColumnMappings().Single().Column, generator.GenerateNext, - scoreProperty.GetTableColumnMappings().Single().TypeMapping, generatedValue, !generatedValue, false, false, true)); - } - - if (concurrencyToken) - { - columnModifications.Add( - new( - entry, versionProperty, versionProperty.GetTableColumnMappings().Single().Column, generator.GenerateNext, - versionProperty.GetTableColumnMappings().Single().TypeMapping, false, false, false, true, true)); - } - - var modificationCommandParameters = new ModificationCommandParameters( - entry.EntityType.GetTableMappings().Single().Table, sensitiveLoggingEnabled: false); - var modificationCommand = CreateMutableModificationCommandFactory().CreateModificationCommand(modificationCommandParameters); - - modificationCommand.AddEntry(entry, mainEntry: true); - - foreach (var columnModification in columnModifications) - { - ((INonTrackedModificationCommand)modificationCommand).AddColumnModification(columnModification); - } - - return modificationCommand; - } - - private IModel GetOptionalFragmentModel() - { - var modelBuilder = TestHelpers.CreateConventionBuilder(); - modelBuilder.Entity().ToTable("CustomerDetails", Schema) - .Property(e => e.Id).ValueGeneratedNever(); - return modelBuilder.Model.FinalizeModel(); - } - - private class OptionalFragmentDetail - { - public int Id { get; set; } - public string Description { get; set; } - public int? Score { get; set; } - public int? Version { get; set; } - } } diff --git a/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs index 7188b63c2d1..ccf4e16dad1 100644 --- a/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs @@ -5,10 +5,10 @@ namespace Microsoft.EntityFrameworkCore; #nullable disable -// Sqlite doesn't implement the conditional upsert/delete required to write optional entity-splitting fragments; -// these tests verify that this is surfaced as a clear provider-capability error rather than silently wrong SQL, -// and that operations which never need to touch the optional fragment (e.g. inserting/deleting an all-null -// fragment) keep working, since the "should we touch this fragment at all" decision is provider-neutral. +// Whether an optional entity-splitting fragment's row needs to be inserted, updated, or left alone when the entity +// is deleted is decided from the entry's tracked original values rather than from provider-specific SQL, so these +// scenarios work identically on every relational provider, including ones like Sqlite that have no special support +// for conditional upserts/deletes. public class OptionalEntitySplittingSqliteTest : NonSharedModelTestBase, IClassFixture { public OptionalEntitySplittingSqliteTest(NonSharedFixture fixture) @@ -47,8 +47,8 @@ public async Task Insert_with_all_null_optional_payload_does_not_touch_fragment_ { context.Customers.Add(new Customer { Id = 1, Name = "Alice" }); - // No exception: the optional-fragment INSERT is skipped entirely (provider-neutral decision) - // because every payload value mapped to it is null. + // No exception: the optional-fragment INSERT is skipped entirely because every payload value mapped + // to it is null. await context.SaveChangesAsync(); } @@ -61,35 +61,130 @@ public async Task Insert_with_all_null_optional_payload_does_not_touch_fragment_ } [Fact] - public async Task Update_of_optional_fragment_throws_provider_capability_error() + public async Task Insert_with_non_null_optional_payload_inserts_fragment_row() { var contextFactory = await InitializeContextAsync(); - await using var context = contextFactory.CreateDbContext(); - context.Customers.Add(new Customer { Id = 1, Name = "Alice" }); - await context.SaveChangesAsync(); + await using (var context = contextFactory.CreateDbContext()) + { + context.Customers.Add(new Customer { Id = 1, Name = "Alice", Description = "Some details" }); + + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + Assert.Equal("Some details", customer.Description); + } + } + + [Fact] + public async Task Setting_previously_absent_optional_fragment_inserts_row() + { + var contextFactory = await InitializeContextAsync(); + + await using (var context = contextFactory.CreateDbContext()) + { + context.Customers.Add(new Customer { Id = 1, Name = "Alice" }); + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + customer.Description = "Some details"; + + // The fragment row was never inserted, so this is treated as an INSERT rather than an UPDATE. + // If it were sent as an UPDATE, it would affect zero rows and throw DbUpdateConcurrencyException. + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + Assert.Equal("Some details", customer.Description); + } + } + + [Fact] + public async Task Updating_previously_present_optional_fragment_updates_row() + { + var contextFactory = await InitializeContextAsync(); + + await using (var context = contextFactory.CreateDbContext()) + { + context.Customers.Add(new Customer { Id = 1, Name = "Alice", Description = "Some details" }); + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + customer.Description = "Other details"; + + // The fragment row already exists, so this must be sent as an UPDATE. If it were sent as an INSERT, + // it would violate the primary key and throw. + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + Assert.Equal("Other details", customer.Description); + } + } + + [Fact] + public async Task Deleting_entity_with_absent_optional_fragment_completes_without_error() + { + var contextFactory = await InitializeContextAsync(); + + await using (var context = contextFactory.CreateDbContext()) + { + context.Customers.Add(new Customer { Id = 1, Name = "Alice" }); + await context.SaveChangesAsync(); + } - var customer = await context.Customers.SingleAsync(); - customer.Description = "Some details"; + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + context.Customers.Remove(customer); + + // The fragment row was never inserted, so its DELETE is skipped. If it were sent, it would affect + // zero rows and throw DbUpdateConcurrencyException. + await context.SaveChangesAsync(); + } - var exception = await Assert.ThrowsAsync(() => context.SaveChangesAsync()); - Assert.Equal(RelationalStrings.OptionalEntitySplittingNotSupported, exception.Message); + await using (var context = contextFactory.CreateDbContext()) + { + Assert.Empty(await context.Customers.ToListAsync()); + } } [Fact] - public async Task Delete_of_entity_with_optional_fragment_throws_provider_capability_error() + public async Task Deleting_entity_with_present_optional_fragment_deletes_row() { var contextFactory = await InitializeContextAsync(); - await using var context = contextFactory.CreateDbContext(); - context.Customers.Add(new Customer { Id = 1, Name = "Alice" }); - await context.SaveChangesAsync(); + await using (var context = contextFactory.CreateDbContext()) + { + context.Customers.Add(new Customer { Id = 1, Name = "Alice", Description = "Some details" }); + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + context.Customers.Remove(customer); - var customer = await context.Customers.SingleAsync(); - context.Customers.Remove(customer); + await context.SaveChangesAsync(); + } - var exception = await Assert.ThrowsAsync(() => context.SaveChangesAsync()); - Assert.Equal(RelationalStrings.OptionalEntitySplittingNotSupported, exception.Message); + await using (var context = contextFactory.CreateDbContext()) + { + Assert.Empty(await context.Customers.ToListAsync()); + } } protected class CustomerContext(DbContextOptions options) : PoolableDbContext(options) From c5f1b5eabecaea3072515512f7b9f1df3d520d00 Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Sat, 1 Aug 2026 14:38:07 +0500 Subject: [PATCH 09/11] Warn when a migration changes an entity-splitting fragment's optionality Part of #27974 --- .../EntityTypeMappingFragmentEventData.cs | 46 ++++++++++ .../Diagnostics/RelationalEventId.cs | 16 ++++ .../Diagnostics/RelationalLoggerExtensions.cs | 46 ++++++++++ .../RelationalLoggingDefinitions.cs | 9 ++ .../EFCore.Relational.baseline.json | 25 ++++++ .../Internal/MigrationsModelDiffer.cs | 27 +++++- .../Properties/RelationalStrings.Designer.cs | 25 ++++++ .../Properties/RelationalStrings.resx | 4 + .../Design/MigrationScaffolderTest.cs | 3 +- .../Internal/MigrationsModelDifferTest.cs | 87 +++++++++++++++++++ .../Internal/MigrationsModelDifferTestBase.cs | 3 +- 11 files changed, 288 insertions(+), 3 deletions(-) create mode 100644 src/EFCore.Relational/Diagnostics/EntityTypeMappingFragmentEventData.cs diff --git a/src/EFCore.Relational/Diagnostics/EntityTypeMappingFragmentEventData.cs b/src/EFCore.Relational/Diagnostics/EntityTypeMappingFragmentEventData.cs new file mode 100644 index 00000000000..47cb2f252e4 --- /dev/null +++ b/src/EFCore.Relational/Diagnostics/EntityTypeMappingFragmentEventData.cs @@ -0,0 +1,46 @@ +// 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.Diagnostics; + +/// +/// The event payload for events that reference an entity-splitting mapping fragment. +/// +public class EntityTypeMappingFragmentEventData : EventData +{ + /// + /// Initializes a new instance of the class. + /// + /// The event definition. + /// A delegate that generates a log message for this event. + /// The entity type that owns the mapping fragment. + /// The store object for the mapping fragment. + /// A value indicating whether the fragment is now optional. + public EntityTypeMappingFragmentEventData( + EventDefinitionBase eventDefinition, + Func messageGenerator, + IEntityType entityType, + StoreObjectIdentifier storeObject, + bool optional) + : base(eventDefinition, messageGenerator) + { + EntityType = entityType; + StoreObject = storeObject; + IsOptional = optional; + } + + /// + /// Gets the entity type that owns the mapping fragment. + /// + public virtual IEntityType EntityType { get; } + + /// + /// Gets the store object for the mapping fragment. + /// + public virtual StoreObjectIdentifier StoreObject { get; } + + /// + /// Gets a value indicating whether the fragment is now optional. + /// + public virtual bool IsOptional { get; } +} diff --git a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs index 2addb102a62..3eaaf8d42b6 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs @@ -84,6 +84,7 @@ private enum Id MigrationsUserTransactionWarning = CoreEventId.RelationalBaseId + 412, ModelSnapshotNotFound = CoreEventId.RelationalBaseId + 413, OldMigrationVersionWarning = CoreEventId.RelationalBaseId + 414, + EntitySplittingFragmentOptionalityChangedWarning = CoreEventId.RelationalBaseId + 415, // Query events QueryClientEvaluationWarning = CoreEventId.RelationalBaseId + 500, @@ -821,6 +822,21 @@ private static EventId MakeMigrationsId(Id id) /// public static readonly EventId OldMigrationVersionWarning = MakeMigrationsId(Id.OldMigrationVersionWarning); + /// + /// The optionality of an entity-splitting fragment changed since the last migration. + /// + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a + /// . + /// + /// + public static readonly EventId EntitySplittingFragmentOptionalityChangedWarning = + MakeMigrationsId(Id.EntitySplittingFragmentOptionalityChangedWarning); + private static readonly string _queryPrefix = DbLoggerCategory.Query.Name + "."; private static EventId MakeQueryId(Id id) diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs index 3ad8ca988d5..39fef2f3690 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs @@ -3700,6 +3700,52 @@ private static string ColumnOrderIgnoredWarning(EventDefinitionBase definition, return d.GenerateMessage((p.ColumnOperation.Table, p.ColumnOperation.Schema).FormatTable(), p.ColumnOperation.Name); } + /// + /// 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. + /// + public static void EntitySplittingFragmentOptionalityChangedWarning( + this IDiagnosticsLogger diagnostics, + IEntityType entityType, + StoreObjectIdentifier storeObject, + bool optional) + { + var definition = RelationalResources.LogEntitySplittingFragmentOptionalityChangedWarning(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log( + diagnostics, + storeObject.DisplayName(), + entityType.DisplayName(), + optional ? "optional" : "required"); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new EntityTypeMappingFragmentEventData( + definition, + EntitySplittingFragmentOptionalityChangedWarning, + entityType, + storeObject, + optional); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string EntitySplittingFragmentOptionalityChangedWarning(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (EntityTypeMappingFragmentEventData)payload; + return d.GenerateMessage( + p.StoreObject.DisplayName(), + p.EntityType.DisplayName(), + p.IsOptional ? "optional" : "required"); + } + /// /// Logs for the event. /// diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs index 90448b21c7b..1efd8b5676c 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs @@ -682,6 +682,15 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions [EntityFrameworkInternal] public EventDefinitionBase? LogColumnOrderIgnoredWarning; + /// + /// 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. + /// + [EntityFrameworkInternal] + public EventDefinitionBase? LogEntitySplittingFragmentOptionalityChangedWarning; + /// /// 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 diff --git a/src/EFCore.Relational/EFCore.Relational.baseline.json b/src/EFCore.Relational/EFCore.Relational.baseline.json index ad6f0467b72..3f581ab6a80 100644 --- a/src/EFCore.Relational/EFCore.Relational.baseline.json +++ b/src/EFCore.Relational/EFCore.Relational.baseline.json @@ -2817,6 +2817,25 @@ } ] }, + { + "Type": "class Microsoft.EntityFrameworkCore.Diagnostics.EntityTypeMappingFragmentEventData : Microsoft.EntityFrameworkCore.Diagnostics.EventData", + "Methods": [ + { + "Member": "EntityTypeMappingFragmentEventData(Microsoft.EntityFrameworkCore.Diagnostics.EventDefinitionBase eventDefinition, System.Func messageGenerator, Microsoft.EntityFrameworkCore.Metadata.IEntityType entityType, Microsoft.EntityFrameworkCore.Metadata.StoreObjectIdentifier storeObject, bool optional);" + } + ], + "Properties": [ + { + "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.IEntityType EntityType { get; }" + }, + { + "Member": "virtual bool IsOptional { get; }" + }, + { + "Member": "virtual Microsoft.EntityFrameworkCore.Metadata.StoreObjectIdentifier StoreObject { get; }" + } + ] + }, { "Type": "class Microsoft.EntityFrameworkCore.Diagnostics.EntityTypeSchemaEventData : Microsoft.EntityFrameworkCore.Diagnostics.EventData", "Methods": [ @@ -12740,6 +12759,9 @@ { "Member": "static readonly Microsoft.Extensions.Logging.EventId DuplicateColumnOrders" }, + { + "Member": "static readonly Microsoft.Extensions.Logging.EventId EntitySplittingFragmentOptionalityChangedWarning" + }, { "Member": "static readonly Microsoft.Extensions.Logging.EventId ExecuteDeleteFailed" }, @@ -13467,6 +13489,9 @@ { "Member": "static void DuplicateColumnOrders(this Microsoft.EntityFrameworkCore.Diagnostics.IDiagnosticsLogger diagnostics, Microsoft.EntityFrameworkCore.Metadata.StoreObjectIdentifier storeObject, System.Collections.Generic.IReadOnlyList columns);" }, + { + "Member": "static void EntitySplittingFragmentOptionalityChangedWarning(this Microsoft.EntityFrameworkCore.Diagnostics.IDiagnosticsLogger diagnostics, Microsoft.EntityFrameworkCore.Metadata.IEntityType entityType, Microsoft.EntityFrameworkCore.Metadata.StoreObjectIdentifier storeObject, bool optional);" + }, { "Member": "static void ExecuteDeleteFailed(this Microsoft.EntityFrameworkCore.Diagnostics.IDiagnosticsLogger diagnostics, System.Type contextType, System.Exception exception);" }, diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs index e2bfbf3b8b5..6c6d620bfe9 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs @@ -56,13 +56,15 @@ public MigrationsModelDiffer( IMigrationsAnnotationProvider migrationsAnnotationProvider, IRelationalAnnotationProvider relationalAnnotationProvider, IRowIdentityMapFactory rowIdentityMapFactory, - CommandBatchPreparerDependencies commandBatchPreparerDependencies) + CommandBatchPreparerDependencies commandBatchPreparerDependencies, + IDiagnosticsLogger logger) { TypeMappingSource = typeMappingSource; MigrationsAnnotationProvider = migrationsAnnotationProvider; RelationalAnnotationProvider = relationalAnnotationProvider; RowIdentityMapFactory = rowIdentityMapFactory; CommandBatchPreparerDependencies = commandBatchPreparerDependencies; + Logger = logger; } /// @@ -105,6 +107,14 @@ public MigrationsModelDiffer( /// protected virtual CommandBatchPreparerDependencies CommandBatchPreparerDependencies { get; } + /// + /// 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. + /// + protected virtual IDiagnosticsLogger Logger { get; } + /// /// 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 @@ -596,6 +606,21 @@ protected virtual IEnumerable Diff( yield break; } + foreach (var sourceMapping in source.EntityTypeMappings) + { + var targetMapping = target.EntityTypeMappings.FirstOrDefault( + m => string.Equals(m.TypeBase.Name, sourceMapping.TypeBase.Name, StringComparison.OrdinalIgnoreCase)); + if (targetMapping != null + && sourceMapping.IsSplitFragmentOptional != targetMapping.IsSplitFragmentOptional + && targetMapping.TypeBase is IEntityType targetEntityType) + { + Logger.EntitySplittingFragmentOptionalityChangedWarning( + targetEntityType, + StoreObjectIdentifier.Table(target.Name, target.Schema), + targetMapping.IsSplitFragmentOptional); + } + } + if (source.Schema != target.Schema || source.Name != target.Name) { diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 29f755dac4f..74b58416e2d 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -3393,6 +3393,31 @@ public static EventDefinition LogDuplicateColumnOrders(IDiagnost return (EventDefinition)definition; } + /// + /// The optionality of the entity-splitting fragment '{storeObject}' for entity type '{entityType}' changed to '{optionality}' since the last migration. This may be a purely behavioral change with no corresponding schema change; review the generated migration to confirm it reflects the intended change. + /// + public static EventDefinition LogEntitySplittingFragmentOptionalityChangedWarning(IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogEntitySplittingFragmentOptionalityChangedWarning; + if (definition == null) + { + definition = NonCapturingLazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogEntitySplittingFragmentOptionalityChangedWarning, + logger, + static logger => new EventDefinition( + logger.Options, + RelationalEventId.EntitySplittingFragmentOptionalityChangedWarning, + LogLevel.Warning, + "RelationalEventId.EntitySplittingFragmentOptionalityChangedWarning", + level => LoggerMessage.Define( + level, + RelationalEventId.EntitySplittingFragmentOptionalityChangedWarning, + _resourceManager.GetString("LogEntitySplittingFragmentOptionalityChangedWarning")!))); + } + + return (EventDefinition)definition; + } + /// /// An exception occurred while executing an 'ExecuteDelete' operation for context type '{contextType}'.{newline}{error} /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 31c67f9be35..957fcba14d0 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -818,6 +818,10 @@ The configured column orders for the table '{table}' contains duplicates. Ensure the specified column order values are distinct. Conflicting columns: {columns}. Error RelationalEventId.DuplicateColumnOrders string string + + The optionality of the entity-splitting fragment '{storeObject}' for entity type '{entityType}' changed to '{optionality}' since the last migration. This may be a purely behavioral change with no corresponding schema change; review the generated migration to confirm it reflects the intended change. + Warning RelationalEventId.EntitySplittingFragmentOptionalityChangedWarning string string string + An exception occurred while executing an 'ExecuteDelete' operation for context type '{contextType}'.{newline}{error} Error RelationalEventId.ExecuteDeleteFailed Type string Exception diff --git a/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs b/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs index 342b9451b2e..ed726df1c06 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs @@ -102,7 +102,8 @@ var migrationAssembly new RelationalAnnotationProvider( new RelationalAnnotationProviderDependencies()), services.GetRequiredService(), - services.GetRequiredService()), + services.GetRequiredService(), + new FakeDiagnosticsLogger()), idGenerator, new MigrationsCodeGeneratorSelector( [ diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs index e8b4304a60c..440edadefd3 100644 --- a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs +++ b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs @@ -1,7 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.EntityFrameworkCore.Diagnostics.Internal; using Microsoft.EntityFrameworkCore.TestUtilities.FakeProvider; +using Microsoft.EntityFrameworkCore.Update.Internal; // ReSharper disable UnusedAutoPropertyAccessor.Local // ReSharper disable ClassNeverInstantiated.Local @@ -1588,6 +1590,91 @@ public void Foreign_key_to_entity_split_principal_ak_on_fragment_points_to_fragm Assert.Equal("Company", m.Name); })); + [Fact] + public void Detects_entity_splitting_fragment_optionality_change() + { + var (sourceModel, targetModel) = BuildCustomerModels(sourceIsOptional: false, targetIsOptional: true); + + var listLoggerFactory = new ListLoggerFactory(_ => true); + var modelDiffer = CreateModelDifferWithLogger(targetModel, listLoggerFactory); + + modelDiffer.GetDifferences(sourceModel.GetRelationalModel(), targetModel.GetRelationalModel()); + + var warning = Assert.Single( + listLoggerFactory.Log, + l => l.Id == RelationalEventId.EntitySplittingFragmentOptionalityChangedWarning); + Assert.Contains("CustomerDetails", warning.Message); + Assert.Contains("Customer", warning.Message); + Assert.Contains("optional", warning.Message); + } + + [Fact] + public void Does_not_warn_when_entity_splitting_fragment_optionality_is_unchanged() + { + var (sourceModel, targetModel) = BuildCustomerModels(sourceIsOptional: false, targetIsOptional: false); + + var listLoggerFactory = new ListLoggerFactory(_ => true); + var modelDiffer = CreateModelDifferWithLogger(targetModel, listLoggerFactory); + + modelDiffer.GetDifferences(sourceModel.GetRelationalModel(), targetModel.GetRelationalModel()); + + Assert.DoesNotContain( + listLoggerFactory.Log, + l => l.Id == RelationalEventId.EntitySplittingFragmentOptionalityChangedWarning); + } + + private (IModel Source, IModel Target) BuildCustomerModels(bool sourceIsOptional, bool targetIsOptional) + { + IModel BuildModel(bool isOptional) + { + var modelBuilder = CreateModelBuilder(skipConventions: false); + modelBuilder.Entity( + "Customer", x => + { + x.Property("Id"); + x.Property("Name"); + x.SplitToTable( + "CustomerDetails", t => + { + if (isOptional) + { + t.IsOptional(); + } + + t.Property("Description"); + }); + }); + + return modelBuilder.FinalizeModel(designTime: true, skipValidation: true); + } + + return (BuildModel(sourceIsOptional), BuildModel(targetIsOptional)); + } + + private MigrationsModelDiffer CreateModelDifferWithLogger(IModel targetModel, ListLoggerFactory listLoggerFactory) + { + var options = new LoggingOptions(); + options.Initialize(new DbContextOptionsBuilder().EnableSensitiveDataLogging().Options); + var logger = new DiagnosticsLogger( + listLoggerFactory, + options, + new DiagnosticListener("Fake"), + TestHelpers.LoggingDefinitions, + new NullDbContextLogger()); + + var targetOptions = TestHelpers.AddProviderOptions(new DbContextOptionsBuilder()).UseModel(targetModel).Options; + + return new MigrationsModelDiffer( + new TestRelationalTypeMappingSource( + TestServiceFactory.Instance.Create(), + TestServiceFactory.Instance.Create()), + new MigrationsAnnotationProvider(new MigrationsAnnotationProviderDependencies()), + new RelationalAnnotationProvider(new RelationalAnnotationProviderDependencies()), + TestServiceFactory.Instance.Create(), + TestHelpers.CreateContext(targetOptions).GetService(), + logger); + } + [Fact] public void Add_owned_types() => Execute( diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTestBase.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTestBase.cs index c53fb05c4fc..fdd6104ef93 100644 --- a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTestBase.cs +++ b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTestBase.cs @@ -127,5 +127,6 @@ protected virtual MigrationsModelDiffer CreateModelDiffer(DbContextOptions optio new RelationalAnnotationProvider( new RelationalAnnotationProviderDependencies()), TestServiceFactory.Instance.Create(), - TestHelpers.CreateContext(options).GetService()); + TestHelpers.CreateContext(options).GetService(), + new FakeDiagnosticsLogger()); } From 7c97afc0ec6ddeef19a5f5e825e82c1ab7607285 Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Thu, 6 Aug 2026 09:16:15 +0500 Subject: [PATCH 10/11] Removed unused imports Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../Update/SqlServerUpdateSqlGeneratorTest.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs index 23663830a89..9d4c06e9d0f 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/SqlServerUpdateSqlGeneratorTest.cs @@ -1,11 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Update.Internal; -using Microsoft.EntityFrameworkCore.Update.Internal; // ReSharper disable InconsistentNaming namespace Microsoft.EntityFrameworkCore.Update; From 8a81fc95058cf78f16f7bffde128ec41f88da056 Mon Sep 17 00:00:00 2001 From: Anas Ismail Khan Date: Thu, 6 Aug 2026 14:40:47 +0500 Subject: [PATCH 11/11] Delete optional entity-splitting fragment rows when their payload goes all-null Part of #27974 --- .../EFCore.Relational.baseline.json | 3 + .../Update/ModificationCommand.cs | 19 +++++ .../OptionalEntitySplittingSqliteTest.cs | 72 ++++++++++++++++++- 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/EFCore.Relational/EFCore.Relational.baseline.json b/src/EFCore.Relational/EFCore.Relational.baseline.json index 18067dc7e57..dc6e2f47381 100644 --- a/src/EFCore.Relational/EFCore.Relational.baseline.json +++ b/src/EFCore.Relational/EFCore.Relational.baseline.json @@ -8561,6 +8561,9 @@ { "Member": "virtual bool IsOptionalSplitFragment { get; }" }, + { + "Member": "virtual bool IsOptionalSplitFragmentPayloadAllNull { get; }" + }, { "Member": "virtual bool IsOptionalSplitFragmentRowAssumedAbsent { get; }" }, diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs index 5ef2b778c28..529aadcc9e6 100644 --- a/src/EFCore.Relational/Update/ModificationCommand.cs +++ b/src/EFCore.Relational/Update/ModificationCommand.cs @@ -116,6 +116,18 @@ public virtual bool IsOptionalSplitFragmentRowAssumedAbsent .Where(m => !m.Property.IsPrimaryKey()) .All(m => _entries[0].GetOriginalValue(m.Property) is null); + /// + /// Gets a value indicating whether this command targets an optional entity-splitting fragment table for which + /// every non-key property mapped to it now has a current value. + /// + public virtual bool IsOptionalSplitFragmentPayloadAllNull + => StoreStoredProcedure is null + && _entries.Count > 0 + && GetTableMapping(_entries[0].EntityType) is { IsSplitFragmentOptional: true } tableMapping + && tableMapping.ColumnMappings + .Where(m => !m.Property.IsPrimaryKey()) + .All(m => _entries[0].GetCurrentValue(m.Property) is null); + /// /// The list of needed to perform the insert, update, or delete. /// @@ -193,6 +205,13 @@ public virtual void AddEntry(IUpdateEntry entry, bool mainEntry) { _entityState = EntityState.Added; } + // Conversely, if the row exists but every non-key value mapped to it is now null, delete it rather than + // leaving behind an all-null row that a later save would misdetect as absent and try to re-insert. + else if (_entityState == EntityState.Modified + && IsOptionalSplitFragmentPayloadAllNull) + { + _entityState = EntityState.Deleted; + } } else { diff --git a/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs index ccf4e16dad1..be9e7890131 100644 --- a/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/OptionalEntitySplittingSqliteTest.cs @@ -5,8 +5,8 @@ namespace Microsoft.EntityFrameworkCore; #nullable disable -// Whether an optional entity-splitting fragment's row needs to be inserted, updated, or left alone when the entity -// is deleted is decided from the entry's tracked original values rather than from provider-specific SQL, so these +// Whether an optional entity-splitting fragment's row needs to be inserted, updated, deleted, or left alone is +// decided from the entry's tracked original and current values rather than from provider-specific SQL, so these // scenarios work identically on every relational provider, including ones like Sqlite that have no special support // for conditional upserts/deletes. public class OptionalEntitySplittingSqliteTest : NonSharedModelTestBase, IClassFixture @@ -135,6 +135,74 @@ public async Task Updating_previously_present_optional_fragment_updates_row() } } + [Fact] + public async Task Clearing_a_present_optional_fragment_back_to_null_deletes_the_row() + { + var contextFactory = await InitializeContextAsync(); + + await using (var context = contextFactory.CreateDbContext()) + { + context.Customers.Add(new Customer { Id = 1, Name = "Alice", Description = "Some details" }); + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + customer.Description = null; + + // Every payload value mapped to the fragment is now null, so the row is deleted rather than left + // behind with an all-null payload that would be indistinguishable from an absent row. + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + // Reading the mapped property back as null isn't proof the row is gone: a LEFT JOIN reads back + // null for Description whether the CustomerDetails row is absent or present-with-null. Check the + // fragment table directly to actually prove the row was deleted rather than updated to all-null. + var detailsRowCount = await context.Database + .SqlQuery($"SELECT COUNT(*) AS Value FROM CustomerDetails WHERE Id = 1") + .SingleAsync(); + Assert.Equal(0, detailsRowCount); + } + } + + [Fact] + public async Task Setting_a_cleared_optional_fragment_back_to_non_null_does_not_throw() + { + var contextFactory = await InitializeContextAsync(); + + await using (var context = contextFactory.CreateDbContext()) + { + context.Customers.Add(new Customer { Id = 1, Name = "Alice", Description = "Some details" }); + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + customer.Description = null; + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + customer.Description = "New details"; + + // Without deleting the row in the previous save, this would be misdetected as still-absent and sent + // as an INSERT, violating the primary key against the row left behind by that save. + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var customer = await context.Customers.SingleAsync(); + Assert.Equal("New details", customer.Description); + } + } + [Fact] public async Task Deleting_entity_with_absent_optional_fragment_completes_without_error() {