Make type mapping generic so that it can create value comparers for NativeAOT#38440
Open
AndriySvyryd wants to merge 2 commits into
Open
Make type mapping generic so that it can create value comparers for NativeAOT#38440AndriySvyryd wants to merge 2 commits into
AndriySvyryd wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves NativeAOT compatibility for compiled models by avoiding reflective creation of default value comparers/converters, and by making type mappings generic where the model CLR type is statically known.
Changes:
- Add generic type-mapping base types (
CoreTypeMapping<T>,RelationalTypeMapping<T>) and update built-in relational mappings to derive from them, enabling non-reflective default comparer creation. - Add cached default instances for common value converters and default comparers (
ValueComparer<T>.Default*, various*.Instance) and adjust codegen/baselines to reuse these instead of emitting lambdas. - Update compiled-model scaffolding baselines and tests to remove redundant comparer emissions and to align with the new type-mapping/converter patterns.
Reviewed changes
Copilot reviewed 148 out of 182 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Vector_index/VectorIndexEntityEntityType.cs | Baseline updated to use default type mapping/comparers instead of emitted lambdas. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Triggers/DataEntityType.cs | Baseline updated to reduce emitted comparer/converter code. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Tpc_Sprocs/DependentBaseEntityType.cs | Baseline updated to rely on default mapping instances. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/SpatialTypesTest/SpatialTypesEntityType.cs | Baseline updated to omit redundant comparer cloning. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/SimpleModel/DependentDerivedEntityType.cs | Baseline updated to avoid explicit comparer construction. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/ManyTypesEntityType.cs | Baseline updated to use DefaultValueComparer<T> instances. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Key_sequence/DataEntityType.cs | Baseline updated to omit explicit default comparer emission. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Key_HiLo_sequence/DataEntityType.cs | Baseline updated to omit explicit default comparer emission. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Full_text_index/FullTextEntityEntityType.cs | Baseline updated to omit redundant comparer/converter code. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Dynamic_schema/DataEntityType.cs | Baseline updated to omit explicit comparer construction. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/DbFunctions/DbFunctionContextModelBuilder.cs | Baseline updated to remove explicit comparers and reuse defaults. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/DbFunctions/DataEntityType.cs | Baseline updated to omit explicit byte[] comparer construction. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Custom_function_type_mapping/FunctionTypeMappingContextModelBuilder.cs | Baseline updated to omit explicit string comparer construction. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Custom_function_parameter_type_mapping/FunctionParameterTypeMappingContextModelBuilder.cs | Baseline updated to omit explicit string comparer construction. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/CheckConstraints/DataEntityType.cs | Baseline updated to omit explicit comparer construction. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/PrincipalBasePrincipalDerivedDependentBasebyteEntityType.cs | Baseline updated to use default comparers (incl. structural comparisons). |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/DependentDerivedEntityType.cs | Baseline updated to omit explicit comparer construction. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/DependentBaseEntityType.cs | Baseline updated to use default enum converters/comparers. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/PrincipalBasePrincipalDerivedDependentBasebyteEntityType.cs | Baseline updated to use default comparers (incl. structural comparisons). |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/DependentDerivedEntityType.cs | Baseline updated to omit explicit comparer construction. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/DependentBaseEntityType.cs | Baseline updated to use default enum converters/comparers. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Triggers/DataEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/SimpleModel/DependentDerivedEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/ManyTypesEntityType.cs | Baseline updated to use DefaultValueComparer<T> instances. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Dynamic_schema/DataEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/DbFunctions/DbFunctionContextModelBuilder.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/DbFunctions/DataEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Custom_function_type_mapping/FunctionTypeMappingContextModelBuilder.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Custom_function_parameter_type_mapping/FunctionParameterTypeMappingContextModelBuilder.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/CheckConstraints/DataEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/PrincipalBasePrincipalDerivedDependentBasebyteEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/DependentBaseEntityType.cs | Baseline updated to use default enum converters/comparers. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/DataEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/AutoIncrementEntityEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/PrincipalBasePrincipalDerivedDependentBasebyteEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/DependentBaseEntityType.cs | Baseline updated to use default enum converters/comparers. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/DataEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/AutoIncrementEntityEntityType.cs | Baseline updated to remove explicit comparer emission. |
| test/EFCore.Specification.Tests/Scaffolding/CompiledModelTestBase.cs | Renames helper method to avoid naming collision/clarify intent in tests. |
| test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs | Updates test setup to avoid type-mapping clrType cloning and use converters instead. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/CompiledModelInMemoryTest.cs | Updates test to use generic InMemoryTypeMapping<T>. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/SimpleModel/DependentDerivedEntityType.cs | Baseline updated to use generic in-memory type mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Self_referential_property/SelfReferentialEntityEntityType.cs | Baseline updated to use generic in-memory mappings and default comparers. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/RelationshipCycles/DependentBaseEntityType.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/ManyTypesEntityType.cs | Baseline updated to use DefaultValueComparer<T> instances. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Manual_lazy_loading/LazyPropertyEntityEntityType.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Manual_lazy_loading/LazyPropertyDelegateEntityEntityType.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Manual_lazy_loading/LazyConstructorEntityEntityType.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Lazy_loading_proxies/LazyProxiesEntity2EntityType.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Lazy_loading_proxies/LazyProxiesEntity1EntityType.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Lazy_loading_manual/LazyProxiesEntity4EntityType.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Lazy_loading_manual/LazyProxiesEntity3EntityType.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Global_namespace/EntityType1.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Fully_qualified_model/ScaffoldingEntityType.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Fully_qualified_model/IndexEntityType.cs | Baseline updated to use generic in-memory mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Custom_value_converter/MyEntityEntityType.cs | Baseline updated to use DefaultValueComparer<T> and generic mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Custom_value_comparer/MyEntityEntityType.cs | Baseline updated to use generic mappings (custom comparer still emitted). |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Custom_type_mapping/MyEntityEntityType.cs | Baseline updated to use generic mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/Custom_provider_value_comparer/MyEntityEntityType.cs | Baseline updated to use generic mappings (custom provider comparer still emitted). |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/BigModel/PrincipalBasePrincipalDerivedDependentBasebyteEntityType.cs | Baseline updated to use generic mappings and JSON reader/writers. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/BigModel/DependentDerivedEntityType.cs | Baseline updated to use generic mappings. |
| test/EFCore.InMemory.FunctionalTests/Scaffolding/Baselines/BigModel/DependentBaseEntityType.cs | Baseline updated to use generic mappings and enum JSON reader/writers. |
| test/EFCore.Cosmos.FunctionalTests/Scaffolding/Baselines/SimpleModel/DependentDerivedEntityType.cs | Baseline updated to use generic Cosmos type mappings. |
| test/EFCore.Cosmos.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/ManyTypesEntityType.cs | Baseline updated to use DefaultValueComparer<T> instances and generic Cosmos mappings. |
| test/EFCore.Cosmos.FunctionalTests/Scaffolding/Baselines/BigModel/DependentDerivedEntityType.cs | Baseline updated to use generic Cosmos type mappings. |
| test/EFCore.Cosmos.FunctionalTests/Scaffolding/Baselines/BigModel/DependentBaseEntityType.cs | Baseline updated to use default converters (GuidToStringConverter, EnumToNumberConverter) and default comparers. |
| src/EFCore/Storage/ValueConversion/UriToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/TimeSpanToTicksConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/TimeSpanToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/TimeOnlyToTicksConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/TimeOnlyToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToUriConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToTimeSpanConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToTimeOnlyConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToNumberConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToGuidConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToEnumConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToDateTimeOffsetConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToDateTimeConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToCharConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToBytesConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/StringToBoolConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/PhysicalAddressToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/PhysicalAddressToBytesConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/NumberToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/NumberToBytesConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/IPAddressToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/IPAddressToBytesConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/GuidToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/GuidToBytesConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/EnumToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/EnumToNumberConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/DateTimeToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/DateTimeToBinaryConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/DateTimeOffsetToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/DateTimeOffsetToBytesConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/DateTimeOffsetToBinaryConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/DateOnlyToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/CharToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/CastingConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/BytesToStringConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/BoolToZeroOneConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore/Storage/ValueConversion/BoolToStringConverter.cs | Adds cached default converter instance and stabilizes default mapping hints instance. |
| src/EFCore/Storage/CoreTypeMapping`1.cs | Introduces generic core type mapping base to create default comparers without reflection. |
| src/EFCore/Storage/CoreTypeMapping.cs | Routes comparer creation through overridable methods and adds AOT-safe suppression points + default-comparer metadata. |
| src/EFCore/Design/Internal/ICSharpRuntimeAnnotationCodeGenerator.cs | Simplifies Create signature to remove explicit comparer overrides. |
| src/EFCore/Design/ICSharpHelper.cs | Updates lambda generation to operate on IPropertyBase to support complex property paths. |
| src/EFCore/ChangeTracking/ValueComparerExtensions.cs | Updates default-comparer detection to match refactored default comparer types. |
| src/EFCore/ChangeTracking/ValueComparer`.cs | Adds cached default comparer instances and uses Array.Clone() to avoid reflective ToArray generation. |
| src/EFCore/ChangeTracking/ValueComparer.cs | Marks non-generic default-comparer creation as dynamic-code-requiring; reuses cached internal comparer instances. |
| src/EFCore.Relational/Storage/RelationalTypeMapping`1.cs | Introduces generic relational mapping base for AOT-safe default comparer creation. |
| src/EFCore.Relational/Storage/RelationalTypeMapping.cs | Avoids unnecessary clones when facets unchanged and removes ability to clone with different model CLR type. |
| src/EFCore.Relational/Storage/UShortTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<ushort>. |
| src/EFCore.Relational/Storage/ULongTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<ulong>. |
| src/EFCore.Relational/Storage/UIntTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<uint>. |
| src/EFCore.Relational/Storage/TimeSpanTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<TimeSpan>. |
| src/EFCore.Relational/Storage/TimeOnlyTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<TimeOnly>. |
| src/EFCore.Relational/Storage/StringTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<string>. |
| src/EFCore.Relational/Storage/ShortTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<short>. |
| src/EFCore.Relational/Storage/SByteTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<sbyte>. |
| src/EFCore.Relational/Storage/LongTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<long>. |
| src/EFCore.Relational/Storage/IntTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<int>. |
| src/EFCore.Relational/Storage/GuidTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<Guid>. |
| src/EFCore.Relational/Storage/FloatTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<float>. |
| src/EFCore.Relational/Storage/DoubleTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<double>. |
| src/EFCore.Relational/Storage/DecimalTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<decimal>. |
| src/EFCore.Relational/Storage/DateTimeTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<DateTime>. |
| src/EFCore.Relational/Storage/DateTimeOffsetTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<DateTimeOffset>. |
| src/EFCore.Relational/Storage/DateOnlyTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<DateOnly>. |
| src/EFCore.Relational/Storage/CharTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<char>. |
| src/EFCore.Relational/Storage/ByteTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<byte>. |
| src/EFCore.Relational/Storage/ByteArrayTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<byte[]>. |
| src/EFCore.Relational/Storage/BoolTypeMapping.cs | Updates mapping to derive from RelationalTypeMapping<bool>. |
| src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs | Emits type-mapping clone arguments more selectively and centralizes comparer emission logic. |
| src/EFCore.InMemory/Storage/Internal/InMemoryTypeMappingSource.cs | Creates common type mappings via generic path and limits reflection fallback for AOT scenarios. |
| src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping`1.cs | Adds generic in-memory type mapping for AOT-safe default comparer creation. |
| src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs | Adjusts ctor visibility to support generic derived mapping type. |
| src/EFCore.Cosmos/Storage/Internal/ReadOnlyMemoryConverter.cs | Adds cached default converter instance and reuses it via DefaultInfo. |
| src/EFCore.Cosmos/Storage/Internal/CosmosTypeMappingSource.cs | Creates common type mappings via generic path and limits reflection fallback for AOT scenarios. |
| src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping`1.cs | Adds generic Cosmos type mapping for AOT-safe default comparer creation. |
| src/EFCore.Cosmos/Storage/Internal/CosmosTimeSpanTypeMapping.cs | Updates mapping to derive from CosmosTypeMapping<TimeSpan>. |
| src/EFCore.Cosmos/Storage/Internal/CosmosTimeOnlyTypeMapping.cs | Updates mapping to derive from CosmosTypeMapping<TimeOnly>. |
AndriySvyryd
commented
Jun 17, 2026
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 148 out of 182 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/EFCore.Relational/Storage/RelationalTypeMapping.cs:484
RelationalTypeMapping.Clone(...)also removed theclrTypeparameter, which is a breaking change for relational providers/extensions that previously relied onClone(clrType: ...)to retarget a mapping. Consider retaining a compatibility overload (potentially[Obsolete]) which forwards to the new implementation, to avoid breaking external providers on upgrade.
/// <summary>
/// Clones the type mapping to update any parameter if needed.
/// </summary>
/// <param name="mappingInfo">The mapping info containing the facets to use.</param>
/// <param name="storeTypePostfix">The new postfix, or <see langword="null" /> to leave unchanged.</param>
/// <param name="converter">The value converter, or <see langword="null" /> to leave unchanged.</param>
/// <param name="comparer">The value comparer, or <see langword="null" /> to leave unchanged.</param>
/// <param name="keyComparer">The key value comparer, or <see langword="null" /> to leave unchanged.</param>
/// <param name="providerValueComparer">The provider value comparer, or <see langword="null" /> to leave unchanged.</param>
/// <param name="elementMapping">The element mapping, or <see langword="null" /> to leave unchanged.</param>
/// <param name="jsonValueReaderWriter">The JSON reader/writer, or <see langword="null" /> to leave unchanged.</param>
/// <returns>The cloned mapping, or the original mapping if no clone was needed.</returns>
public virtual RelationalTypeMapping Clone(
in RelationalTypeMappingInfo? mappingInfo = null,
ValueConverter? converter = null,
ValueComparer? comparer = null,
ValueComparer? keyComparer = null,
ValueComparer? providerValueComparer = null,
CoreTypeMapping? elementMapping = null,
JsonValueReaderWriter? jsonValueReaderWriter = null,
StoreTypePostfix? storeTypePostfix = null)
{
AndriySvyryd
commented
Jun 17, 2026
| await base.Parameter_collection_Count_with_huge_number_of_values_over_2_operations_same_parameter_different_type_mapping(); | ||
|
|
||
| Assert.Contains("OPENJSON(@ids) WITH ([Value] int '$')", Fixture.TestSqlLoggerFactory.SqlStatements[0], StringComparison.Ordinal); | ||
| Assert.Contains("@ints1=", Fixture.TestSqlLoggerFactory.SqlStatements[0], StringComparison.Ordinal); |
AndriySvyryd
commented
Jun 17, 2026
| await base.Parameter_collection_Count_with_huge_number_of_values_over_2_operations_same_parameter_different_type_mapping(); | ||
|
|
||
| Assert.Contains("VALUES (2)", Fixture.TestSqlLoggerFactory.SqlStatements[0], StringComparison.Ordinal); | ||
| Assert.Contains("@ids1=", Fixture.TestSqlLoggerFactory.SqlStatements[0], StringComparison.Ordinal); |
Contributor
There was a problem hiding this comment.
This does not look right. In the previous the value 2 was constantized into the query, now it looks like it's a parameter.
Member
Author
There was a problem hiding this comment.
@cincuranet Yes, but your fix relied on type mappings being referentially different even though they represent the same type. You even call out that this was a fragile approach at https://github.com/dotnet/efcore/pull/38440/changes#diff-47dbbe3f0a07188b8248ede2d51e8299d502f671dc588de4561a650a598203e7L665
Member
Author
There was a problem hiding this comment.
Pushed a new fix for the original issue (#37185) as a separate commit. The new heuristic leans more toward OPENJSON
cincuranet
reviewed
Jun 19, 2026
Comment on lines
+255
to
+276
| => clrType switch | ||
| { | ||
| _ when clrType == typeof(bool) => Create<bool>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(byte) => Create<byte>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(sbyte) => Create<sbyte>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(char) => Create<char>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(short) => Create<short>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(ushort) => Create<ushort>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(int) => Create<int>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(uint) => Create<uint>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(long) => Create<long>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(ulong) => Create<ulong>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(float) => Create<float>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(double) => Create<double>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(decimal) => Create<decimal>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(string) => Create<string>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(Guid) => Create<Guid>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(DateTime) => Create<DateTime>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(DateTimeOffset) => Create<DateTimeOffset>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ when clrType == typeof(DateOnly) => Create<DateOnly>(comparer, keyComparer, elementMapping, jsonValueReaderWriter), | ||
| _ => CreateMappingWithReflection(clrType, comparer, keyComparer, elementMapping, jsonValueReaderWriter) | ||
| }; |
cincuranet
reviewed
Jun 19, 2026
| """ | ||
| UPDATE [o] | ||
| SET [o].[Title] = COALESCE([o].[Title], N'') + N'_Suffix' | ||
| SET [o].[Title] = ISNULL([o].[Title], N'') + N'_Suffix' |
Contributor
There was a problem hiding this comment.
Are the changes to ISNULL expected? There's a small change in how datatypes are handled in ISNULL and COALESCE, but for our case there should be no difference, I guess.
Member
Author
There was a problem hiding this comment.
The conversion to ISNULL is an optimization that now can be applied more consistently
| await base.Parameter_collection_Count_with_huge_number_of_values_over_2_operations_same_parameter_different_type_mapping(); | ||
|
|
||
| Assert.Contains("VALUES (2)", Fixture.TestSqlLoggerFactory.SqlStatements[0], StringComparison.Ordinal); | ||
| Assert.Contains("@ids1=", Fixture.TestSqlLoggerFactory.SqlStatements[0], StringComparison.Ordinal); |
Contributor
There was a problem hiding this comment.
This does not look right. In the previous the value 2 was constantized into the query, now it looks like it's a parameter.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 178 out of 230 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
test/EFCore.Specification.Tests/Query/PrimitiveCollectionsQueryTestBase.cs:666
- The base test method was renamed, but some provider test classes still override the old method names, which will cause compilation failures (no suitable method to override). For example:
- test/EFCore.Sqlite.FunctionalTests/Query/PrimitiveCollectionsQuerySqliteTest.cs still overrides
...different_type_mapping() - test/EFCore.Cosmos.FunctionalTests/Query/PrimitiveCollectionsQueryCosmosTest.cs still overrides
...different_type_mapping()
These overrides should be renamed to match the new ...different_property() methods (both the Count and Contains variants).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 179 out of 232 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/EFCore.Relational/Storage/RelationalTypeMapping.cs:483
- RelationalTypeMapping.Clone is a public API, and removing the
clrTypeparameter is a breaking change for providers/extensions that relied on changing the model CLR type without introducing a converter. This also appears to force test-only workarounds (e.g. adding a dummy converter just to carry a different CLR type). Consider preserving this capability via an alternative API (or otherwise providing a supported migration path) while still enabling the NativeAOT-friendly generic type-mapping path.
public virtual RelationalTypeMapping Clone(
in RelationalTypeMappingInfo? mappingInfo = null,
ValueConverter? converter = null,
ValueComparer? comparer = null,
ValueComparer? keyComparer = null,
ValueComparer? providerValueComparer = null,
CoreTypeMapping? elementMapping = null,
JsonValueReaderWriter? jsonValueReaderWriter = null,
StoreTypePostfix? storeTypePostfix = null)
…ativeAOT Add static instance properties for values comparers and converts with default parameters Fixes #36817
Comment on lines
+512
to
516
| // Mark the parameter as visited so the subsequent child visit on this SqlParameterExpression (which would | ||
| // hit the generic SqlParameterExpression branch and add 1 to Count) is suppressed. | ||
| _visitedSqlParameters.Add(sqlParameterExpression); | ||
|
|
||
| switch (sqlParameterExpression.TranslationMode ?? collectionParameterTranslationMode) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add static instance properties for values comparers and converts with default parameters
Fixes #36817