From 9625dbe477bb9a5883580c50205ebbd53ee947a6 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Fri, 3 Mar 2023 19:25:56 +0000 Subject: [PATCH] Allow set operations for columns that map to same store type Fixes #29020 --- .../Query/QuerySqlGenerator.cs | 4 +--- .../Query/SqlExpressions/SelectExpression.cs | 5 +---- .../Storage/RelationalTypeMapping.cs | 18 ++++++++++++++++++ src/EFCore/Storage/CoreTypeMapping.cs | 10 ++++++++++ .../Query/NorthwindQuerySqlServerFixture.cs | 2 +- ...NorthwindSetOperationsQuerySqlServerTest.cs | 2 +- 6 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/EFCore.Relational/Query/QuerySqlGenerator.cs b/src/EFCore.Relational/Query/QuerySqlGenerator.cs index 7e25529c7a6..29f2249eb29 100644 --- a/src/EFCore.Relational/Query/QuerySqlGenerator.cs +++ b/src/EFCore.Relational/Query/QuerySqlGenerator.cs @@ -615,9 +615,7 @@ protected override Expression VisitSqlParameter(SqlParameterExpression sqlParame p => p.InvariantName == parameterName && p is TypeMappedRelationalParameter typeMappedRelationalParameter - && string.Equals( - typeMappedRelationalParameter.RelationalTypeMapping.StoreType, sqlParameterExpression.TypeMapping!.StoreType, - StringComparison.OrdinalIgnoreCase) + && typeMappedRelationalParameter.RelationalTypeMapping.MapsToSameStoreType(sqlParameterExpression.TypeMapping!) && typeMappedRelationalParameter.RelationalTypeMapping.Converter == sqlParameterExpression.TypeMapping!.Converter); if (parameter is null) diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index e432ebc46d8..7e45f2ef3e6 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -2334,10 +2334,7 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi var innerColumn2 = (SqlExpression)expression2; // For now, make sure that both sides output the same store type, otherwise the query may fail. // TODO: with #15586 we'll be able to also allow different store types which are implicitly convertible to one another. - if (!string.Equals( - innerColumn1.TypeMapping!.StoreType, - innerColumn2.TypeMapping!.StoreType, - StringComparison.OrdinalIgnoreCase)) + if (!innerColumn1.TypeMapping!.MapsToSameStoreType(innerColumn2.TypeMapping!)) { throw new InvalidOperationException(RelationalStrings.SetOperationsOnDifferentStoreTypes); } diff --git a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs index 4b52c0a8748..d3f09c4015f 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs @@ -627,4 +627,22 @@ public static MethodInfo GetDataReaderMethod(Type type) /// The expression with customization added. public virtual Expression CustomizeDataReaderExpression(Expression expression) => expression; + + /// + public override bool MapsToSameStoreType(CoreTypeMapping otherMapping) + { + if (!base.MapsToSameStoreType(otherMapping)) + { + return false; + } + + var otherRelationalMapping = ((RelationalTypeMapping)otherMapping); + return + Size == otherRelationalMapping.Size + && Precision == otherRelationalMapping.Precision + && DbType == otherRelationalMapping.DbType + && Scale == otherRelationalMapping.Scale + && IsUnicode == otherRelationalMapping.IsUnicode + && IsFixedLength == otherRelationalMapping.IsFixedLength; + } } diff --git a/src/EFCore/Storage/CoreTypeMapping.cs b/src/EFCore/Storage/CoreTypeMapping.cs index bc39feacff8..698538adf6c 100644 --- a/src/EFCore/Storage/CoreTypeMapping.cs +++ b/src/EFCore/Storage/CoreTypeMapping.cs @@ -224,4 +224,14 @@ public virtual ValueComparer ProviderValueComparer /// An expression tree that can be used to generate code for the literal value. public virtual Expression GenerateCodeLiteral(object value) => throw new NotSupportedException(CoreStrings.LiteralGenerationNotSupported(ClrType.ShortDisplayName())); + + /// + /// Determines whether this type mapping is to the same store type as the given mapping. + /// + /// The other type mapping. + /// if the two mappings map to the same type, otherwise. + public virtual bool MapsToSameStoreType(CoreTypeMapping otherMapping) + => GetType() == otherMapping.GetType() + && (Converter?.ProviderClrType ?? ClrType) + == (otherMapping.Converter?.ProviderClrType ?? otherMapping.ClrType); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindQuerySqlServerFixture.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindQuerySqlServerFixture.cs index 45288212ae3..240c1a24b38 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindQuerySqlServerFixture.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindQuerySqlServerFixture.cs @@ -21,7 +21,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con b.Property(c => c.CustomerID).HasColumnType("nchar(5)"); b.Property(cm => cm.CompanyName).HasMaxLength(40); b.Property(cm => cm.ContactName).HasMaxLength(30); - b.Property(cm => cm.ContactTitle).HasColumnType("NVarChar(30)"); + b.Property(cm => cm.ContactTitle).HasColumnType("national character varying(30)"); }); modelBuilder.Entity( diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSetOperationsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSetOperationsQuerySqlServerTest.cs index 1e4a3682370..33d1486f449 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSetOperationsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSetOperationsQuerySqlServerTest.cs @@ -1344,7 +1344,7 @@ public virtual async Task Union_with_different_store_types_throws(bool async) [ConditionalTheory] // Issue #29020 [MemberData(nameof(IsAsyncData))] - public virtual async Task Union_with_store_types_differing_only_by_case(bool async) + public virtual async Task Union_with_type_mappings_to_same_store_type(bool async) { await AssertQuery( async,