From 368e4b1f4db52a0774738b335049508555e276c4 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Mon, 13 Jan 2020 20:32:49 +0100 Subject: [PATCH 1/4] Remove caching of mappings in NpgsqlTypeMappingSource Caching is already done by TypeMappingSource in EF --- .../Internal/NpgsqlTypeMappingSource.cs | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs index 4ce8b639f..e9fdc4aec 100644 --- a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs +++ b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs @@ -301,14 +301,11 @@ void SetupEnumMappings([NotNull] ISqlGenerationHelper sqlGenerationHelper) protected override RelationalTypeMapping FindMapping(in RelationalTypeMappingInfo mappingInfo) => // First, try any plugins, allowing them to override built-in mappings (e.g. NodaTime) base.FindMapping(mappingInfo) ?? - // Then, any mappings that have already been set up - FindExistingMapping(mappingInfo) ?? - // Try any array mappings which have not yet been set up + FindBaseMapping(mappingInfo) ?? FindArrayMapping(mappingInfo) ?? - // Try any user-defined range mappings which have not yet been set up FindUserRangeMapping(mappingInfo); - protected virtual RelationalTypeMapping FindExistingMapping(in RelationalTypeMappingInfo mappingInfo) + protected virtual RelationalTypeMapping FindBaseMapping(in RelationalTypeMappingInfo mappingInfo) { var clrType = mappingInfo.ClrType; var storeTypeName = mappingInfo.StoreTypeName; @@ -428,7 +425,7 @@ protected virtual RelationalTypeMapping FindArrayMapping(in RelationalTypeMappin new NpgsqlArrayListTypeMapping(storeType, elementMapping) }); Debug.Assert(added); - var mapping = FindExistingMapping(mappingInfo); + var mapping = FindBaseMapping(mappingInfo); Debug.Assert(mapping != null); return mapping; } @@ -453,7 +450,7 @@ protected virtual RelationalTypeMapping FindArrayMapping(in RelationalTypeMappin if (elementMapping is NpgsqlArrayTypeMapping) return null; - return ClrTypeMappings.GetOrAdd(clrType, new NpgsqlArrayArrayTypeMapping(elementMapping, clrType)); + return new NpgsqlArrayArrayTypeMapping(elementMapping, clrType); } if (clrType.IsGenericList()) @@ -469,7 +466,7 @@ protected virtual RelationalTypeMapping FindArrayMapping(in RelationalTypeMappin if (elementMapping is NpgsqlArrayTypeMapping) return null; - return ClrTypeMappings.GetOrAdd(clrType, new NpgsqlArrayListTypeMapping(elementMapping, clrType)); + return new NpgsqlArrayListTypeMapping(elementMapping, clrType); } return null; @@ -525,13 +522,7 @@ protected virtual RelationalTypeMapping FindUserRangeMapping(in RelationalTypeMa if (subtypeMapping == null) throw new Exception($"Could not map range {rangeDefinition.RangeName}, no mapping was found its subtype"); - // Finally, construct a range mapping and add it to our lookup dictionaries - next time it will be found as - // an existing mapping - var rangeMapping = new NpgsqlRangeTypeMapping(rangeDefinition.RangeName, rangeDefinition.SchemaName, rangeClrType, subtypeMapping, _sqlGenerationHelper); - StoreTypeMappings[rangeMapping.StoreType] = new RelationalTypeMapping[] { rangeMapping }; - ClrTypeMappings[rangeMapping.ClrType] = rangeMapping; - - return rangeMapping; + return new NpgsqlRangeTypeMapping(rangeDefinition.RangeName, rangeDefinition.SchemaName, rangeClrType, subtypeMapping, _sqlGenerationHelper); } // We override to support parsing array store names (e.g. varchar(32)[]), timestamp(5) with time zone, etc. From c7e4a75636cb6f95adbc4c39a60b247703ed5166 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Mon, 13 Jan 2020 23:47:21 +0100 Subject: [PATCH 2/4] Fix mapping issues around arrays and byte arrays Fixes #1189 --- .../Extensions/TypeExtensions.cs | 27 +++++++ .../NpgsqlNodaTimeTypeMappingSourcePlugin.cs | 73 +++++++++++-------- src/EFCore.PG/Extensions/TypeExtensions.cs | 12 +++ .../Internal/NpgsqlArrayTranslator.cs | 34 ++++----- .../Internal/NpgsqlSqlExpressionFactory.cs | 10 ++- .../Mapping/NpgsqlArrayArrayTypeMapping.cs | 10 ++- .../Mapping/NpgsqlArrayListTypeMapping.cs | 5 +- .../Mapping/NpgsqlArrayTypeMapping.cs | 37 +++++++++- .../Internal/NpgsqlTypeMappingSource.cs | 63 +++++++++------- .../Query/ArrayArrayQueryTest.cs | 63 +++++++++++----- .../Query/EnumQueryTest.cs | 65 +++++++++++++++-- .../Northwind/NorthwindNpgsqlContext.cs | 29 -------- 12 files changed, 298 insertions(+), 130 deletions(-) create mode 100644 src/EFCore.PG.NodaTime/Extensions/TypeExtensions.cs diff --git a/src/EFCore.PG.NodaTime/Extensions/TypeExtensions.cs b/src/EFCore.PG.NodaTime/Extensions/TypeExtensions.cs new file mode 100644 index 000000000..a30f1ad04 --- /dev/null +++ b/src/EFCore.PG.NodaTime/Extensions/TypeExtensions.cs @@ -0,0 +1,27 @@ +using System; +using System.Collections.Generic; + +#nullable enable + +// ReSharper disable once CheckNamespace +namespace Npgsql.EntityFrameworkCore.PostgreSQL +{ + internal static class TypeExtensions + { + internal static bool IsGenericList(this Type type) + => type.IsGenericType && type.GetGenericTypeDefinition() == typeof(List<>); + + internal static bool IsArrayOrGenericList(this Type type) + => type.IsArray || type.IsGenericList(); + + internal static bool TryGetElementType(this Type type, out Type? elementType) + { + elementType = type.IsArray + ? type.GetElementType() + : type.IsGenericList() + ? type.GetGenericArguments()[0] + : null; + return elementType != null; + } + } +} diff --git a/src/EFCore.PG.NodaTime/Storage/Internal/NpgsqlNodaTimeTypeMappingSourcePlugin.cs b/src/EFCore.PG.NodaTime/Storage/Internal/NpgsqlNodaTimeTypeMappingSourcePlugin.cs index 1999e9ab0..7535c7f93 100644 --- a/src/EFCore.PG.NodaTime/Storage/Internal/NpgsqlNodaTimeTypeMappingSourcePlugin.cs +++ b/src/EFCore.PG.NodaTime/Storage/Internal/NpgsqlNodaTimeTypeMappingSourcePlugin.cs @@ -137,41 +137,55 @@ protected virtual RelationalTypeMapping FindExistingMapping(in RelationalTypeMap : mapping; } - RelationalTypeMapping FindArrayMapping(in RelationalTypeMappingInfo mappingInfo) + // TODO: This is duplicated from NpgsqlTypeMappingSource + protected virtual RelationalTypeMapping FindArrayMapping(in RelationalTypeMappingInfo mappingInfo) { - // PostgreSQL array type names are the element plus [] + var clrType = mappingInfo.ClrType; + Type elementClrType = null; + + if (clrType != null && !clrType.TryGetElementType(out elementClrType)) + return null; // Not an array/list + var storeType = mappingInfo.StoreTypeName; + var storeTypeNameBase = mappingInfo.StoreTypeNameBase; if (storeType != null) { + // PostgreSQL array type names are the element plus [] if (!storeType.EndsWith("[]")) return null; - // Note that we scaffold PostgreSQL arrays to C# arrays, not lists (which are also supported) - - // TODO: In theory support the multiple mappings just like we do with scalars above - // (e.g. DateTimeOffset[] vs. DateTime[] - // TODO: In theory we should parse the element store type to get the base type. This is problematic in plugins - // as we have no access to the RelationalTypeMappingSource, where all this happens var elementStoreType = storeType.Substring(0, storeType.Length - 2); - var elementMapping = FindExistingMapping(new RelationalTypeMappingInfo(elementStoreType, elementStoreType, - mappingInfo.IsUnicode, mappingInfo.Size, mappingInfo.Precision, mappingInfo.Scale)); + var elementStoreTypeNameBase = storeTypeNameBase.Substring(0, storeType.Length - 2); + + RelationalTypeMapping elementMapping; - if (elementMapping != null) + if (elementClrType == null) + { + elementMapping = FindMapping(new RelationalTypeMappingInfo( + elementStoreType, elementStoreTypeNameBase, + mappingInfo.IsUnicode, mappingInfo.Size, mappingInfo.Precision, mappingInfo.Scale)); + } + else { - var added = StoreTypeMappings.TryAdd(storeType, - new RelationalTypeMapping[] - { - new NpgsqlArrayArrayTypeMapping(storeType, elementMapping), - new NpgsqlArrayListTypeMapping(storeType, elementMapping) - }); - Debug.Assert(added); - var mapping = FindExistingMapping(mappingInfo); - Debug.Assert(mapping != null); - return mapping; + elementMapping = FindMapping(new RelationalTypeMappingInfo( + elementClrType, elementStoreType, elementStoreTypeNameBase, + mappingInfo.IsKeyOrIndex, mappingInfo.IsUnicode, mappingInfo.Size, mappingInfo.IsRowVersion, + mappingInfo.IsFixedLength, mappingInfo.Precision, mappingInfo.Scale)); + + // If an element mapping was found only with the help of a value converter, return null and EF will + // construct the corresponding array mapping with a value converter. + if (elementMapping?.Converter != null) + return null; } + + // If no mapping was found for the element, there's no mapping for the array. + // Also, arrays of arrays aren't supported (as opposed to multidimensional arrays) by PostgreSQL + if (elementMapping == null || elementMapping is NpgsqlArrayTypeMapping) + return null; + + return new NpgsqlArrayArrayTypeMapping(storeType, elementMapping); } - var clrType = mappingInfo.ClrType; if (clrType == null) return null; @@ -180,24 +194,25 @@ RelationalTypeMapping FindArrayMapping(in RelationalTypeMappingInfo mappingInfo) var elementType = clrType.GetElementType(); Debug.Assert(elementType != null, "Detected array type but element type is null"); - // If an element isn't supported, neither is its array - var elementMapping = FindExistingMapping(new RelationalTypeMappingInfo(elementType)); - if (elementMapping == null) + // If an element isn't supported, neither is its array. If the element is only supported via value + // conversion we also don't support it. + var elementMapping = FindMapping(new RelationalTypeMappingInfo(elementType)); + if (elementMapping == null || elementMapping.Converter != null) return null; // Arrays of arrays aren't supported (as opposed to multidimensional arrays) by PostgreSQL if (elementMapping is NpgsqlArrayTypeMapping) return null; - return ClrTypeMappings.GetOrAdd(clrType, new NpgsqlArrayArrayTypeMapping(elementMapping, clrType)); + return new NpgsqlArrayArrayTypeMapping(elementMapping, clrType); } - if (clrType.IsGenericType && clrType.GetGenericTypeDefinition() == typeof(List<>)) + if (clrType.IsGenericList()) { var elementType = clrType.GetGenericArguments()[0]; // If an element isn't supported, neither is its array - var elementMapping = FindExistingMapping(new RelationalTypeMappingInfo(elementType)); + var elementMapping = FindMapping(new RelationalTypeMappingInfo(elementType)); if (elementMapping == null) return null; @@ -205,7 +220,7 @@ RelationalTypeMapping FindArrayMapping(in RelationalTypeMappingInfo mappingInfo) if (elementMapping is NpgsqlArrayTypeMapping) return null; - return ClrTypeMappings.GetOrAdd(clrType, new NpgsqlArrayListTypeMapping(elementMapping, clrType)); + return new NpgsqlArrayListTypeMapping(elementMapping, clrType); } return null; diff --git a/src/EFCore.PG/Extensions/TypeExtensions.cs b/src/EFCore.PG/Extensions/TypeExtensions.cs index c02bf4d81..a30f1ad04 100644 --- a/src/EFCore.PG/Extensions/TypeExtensions.cs +++ b/src/EFCore.PG/Extensions/TypeExtensions.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; +#nullable enable + // ReSharper disable once CheckNamespace namespace Npgsql.EntityFrameworkCore.PostgreSQL { @@ -11,5 +13,15 @@ internal static bool IsGenericList(this Type type) internal static bool IsArrayOrGenericList(this Type type) => type.IsArray || type.IsGenericList(); + + internal static bool TryGetElementType(this Type type, out Type? elementType) + { + elementType = type.IsArray + ? type.GetElementType() + : type.IsGenericList() + ? type.GetGenericArguments()[0] + : null; + return elementType != null; + } } } diff --git a/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlArrayTranslator.cs b/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlArrayTranslator.cs index 6945308cf..8f15e428a 100644 --- a/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlArrayTranslator.cs +++ b/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlArrayTranslator.cs @@ -60,15 +60,8 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO return null; var operand = arguments[0]; - - var operandElementType = operand.Type.IsArray - ? operand.Type.GetElementType() - : operand.Type.IsGenericList() - ? operand.Type.GetGenericArguments()[0] - : null; - - if (operandElementType == null) // Not an array/list - return null; + if (!operand.Type.TryGetElementType(out var operandElementType)) + return null; // Not an array/list // Even if the CLR type is an array/list, it may be mapped to a non-array database type (e.g. via value converters). if (operand.TypeMapping is RelationalTypeMapping typeMapping && @@ -97,30 +90,37 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO // since non-PG SQL does not support arrays. If the list is a constant we leave it for regular IN // (functionality the same but more familiar). - // Note: we exclude constant array expressions from this PG-specific optimization since the general - // EF Core mechanism is fine for that case. After https://github.com/aspnet/EntityFrameworkCore/issues/16375 - // is done we may not need the check any more. - // Note: we exclude arrays/lists over Nullable since the ADO layer doesn't handle them (but will in 5.0) - if (method.IsClosedFormOf(Contains) && - _sqlExpressionFactory.FindMapping(operand.Type) != null && + // Exclude constant array expressions from this PG-specific optimization since the general + // EF Core mechanism is fine for that case. After https://github.com/aspnet/EntityFrameworkCore/issues/16375 + // is done we may not need the check any more. !(operand is SqlConstantExpression) && + ( + // Handle either parameters (no mapping but supported CLR type), or array columns. We specifically + // don't want to translate if the type mapping is bytea (CLR type is array, but not an array in + // the database). + operand.TypeMapping == null && _sqlExpressionFactory.FindMapping(operand.Type) != null || + operand.TypeMapping is NpgsqlArrayTypeMapping + ) && + // Exclude arrays/lists over Nullable since the ADO layer doesn't handle them (but will in 5.0) Nullable.GetUnderlyingType(operandElementType) == null) { var item = arguments[1]; + var anyAll = _sqlExpressionFactory.ArrayAnyAll(item, operand, ArrayComparisonType.Any, "="); + // TODO: no null semantics is implemented here (see https://github.com/npgsql/efcore.pg/issues/1142) // We require a null semantics check in case the item is null and the array contains a null. // Advanced parameter sniffing would help here: https://github.com/aspnet/EntityFrameworkCore/issues/17598 // We need to coalesce to false since 'x' = ANY ({'y', NULL}) returns null, not false // (and so will be null when negated too) return _sqlExpressionFactory.OrElse( - _sqlExpressionFactory.ArrayAnyAll(item, operand, ArrayComparisonType.Any, "="), + anyAll, _sqlExpressionFactory.AndAlso( _sqlExpressionFactory.IsNull(item), _sqlExpressionFactory.IsNotNull( _sqlExpressionFactory.Function( "array_position", - new[] { operand, _sqlExpressionFactory.Fragment("NULL") }, + new[] { anyAll.Array, _sqlExpressionFactory.Fragment("NULL") }, typeof(int))))); } diff --git a/src/EFCore.PG/Query/Internal/NpgsqlSqlExpressionFactory.cs b/src/EFCore.PG/Query/Internal/NpgsqlSqlExpressionFactory.cs index 9567eb212..894973b22 100644 --- a/src/EFCore.PG/Query/Internal/NpgsqlSqlExpressionFactory.cs +++ b/src/EFCore.PG/Query/Internal/NpgsqlSqlExpressionFactory.cs @@ -181,13 +181,19 @@ SqlExpression ApplyTypeMappingOnRegexMatch(RegexMatchExpression regexMatchExpres SqlExpression ApplyTypeMappingOnArrayAnyAll(ArrayAnyAllExpression arrayAnyAllExpression) { // Attempt type inference either from the operand to the array or the other way around - var arrayMapping = arrayAnyAllExpression.Array.TypeMapping as NpgsqlArrayTypeMapping; + var arrayMapping = (NpgsqlArrayTypeMapping)arrayAnyAllExpression.Array.TypeMapping; var operandMapping = arrayAnyAllExpression.Operand.TypeMapping ?? arrayMapping?.ElementMapping ?? _typeMappingSource.FindMapping(arrayAnyAllExpression.Operand.Type); - arrayMapping ??= (NpgsqlArrayTypeMapping)_typeMappingSource.FindMapping(arrayAnyAllExpression.Operand.Type.MakeArrayType()); + // Note that we provide both the array CLR type *and* an array store type constructed from the element's + // store type. If we use only the array CLR type, byte[] will yield bytea which we don't want. + arrayMapping ??= (NpgsqlArrayTypeMapping)_typeMappingSource.FindMapping( + arrayAnyAllExpression.Array.Type, operandMapping.StoreType + "[]"); + + if (operandMapping == null || arrayMapping == null) + throw new InvalidOperationException("Couldn't find array or element type mapping in ArrayAnyAllExpression"); return new ArrayAnyAllExpression( ApplyTypeMapping(arrayAnyAllExpression.Operand, operandMapping), diff --git a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayArrayTypeMapping.cs b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayArrayTypeMapping.cs index 54b534586..7ba083a6e 100644 --- a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayArrayTypeMapping.cs +++ b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayArrayTypeMapping.cs @@ -1,5 +1,5 @@ using System; -using System.Text; +using System.Collections.Generic; using Microsoft.EntityFrameworkCore.Storage; using System.Diagnostics; using Microsoft.EntityFrameworkCore.ChangeTracking; @@ -10,8 +10,12 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping /// Maps PostgreSQL arrays to .NET arrays. Only single-dimensional arrays are supported. /// /// - /// Note that mapping PostgreSQL arrays to .NET List{T} is also supported via . - /// See: https://www.postgresql.org/docs/current/static/arrays.html + /// + /// Note that mapping PostgreSQL arrays to .NET is also supported via + /// . + /// + /// + /// See: https://www.postgresql.org/docs/current/static/arrays.html /// public class NpgsqlArrayArrayTypeMapping : NpgsqlArrayTypeMapping { diff --git a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayListTypeMapping.cs b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayListTypeMapping.cs index 1ede30662..fdf9e199e 100644 --- a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayListTypeMapping.cs +++ b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayListTypeMapping.cs @@ -11,8 +11,11 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping /// Maps PostgreSQL arrays to . /// /// + /// /// Note that mapping PostgreSQL arrays to .NET arrays is also supported via . - /// See: https://www.postgresql.org/docs/current/static/arrays.html + /// + /// + /// See: https://www.postgresql.org/docs/current/static/arrays.html /// public class NpgsqlArrayListTypeMapping : NpgsqlArrayTypeMapping { diff --git a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayTypeMapping.cs b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayTypeMapping.cs index 650ff1377..11a573ce0 100644 --- a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayTypeMapping.cs +++ b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayTypeMapping.cs @@ -1,8 +1,12 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Data.Common; using System.Text; using Microsoft.EntityFrameworkCore.Storage; +using NpgsqlTypes; + +#nullable enable namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping { @@ -19,9 +23,27 @@ public abstract class NpgsqlArrayTypeMapping : RelationalTypeMapping /// public RelationalTypeMapping ElementMapping { get; } + /// + /// The database type used by Npgsql. + /// + public NpgsqlDbType? NpgsqlDbType { get; } + protected NpgsqlArrayTypeMapping(RelationalTypeMappingParameters parameters, RelationalTypeMapping elementMapping) : base(parameters) - => ElementMapping = elementMapping; + { + ElementMapping = elementMapping; + + // If the element mapping has an NpgsqlDbType or DbType, set our own NpgsqlDbType as an array of that. + // Otherwise let the ADO.NET layer infer the PostgreSQL type. We can't always let it infer, otherwise + // when given a byte[] it will infer byte (but we want smallint[]) + var elementNpgsqlDbType = elementMapping is NpgsqlTypeMapping elementNpgsqlTypeMapping + ? elementNpgsqlTypeMapping.NpgsqlDbType + : elementMapping.DbType.HasValue + ? new NpgsqlParameter { DbType = elementMapping.DbType.Value }.NpgsqlDbType + : (NpgsqlDbType?)null; + if (elementNpgsqlDbType != null) + NpgsqlDbType = elementNpgsqlDbType | NpgsqlTypes.NpgsqlDbType.Array; + } // The array-to-array mapping needs to know how to generate an SQL literal for a List<>, and // the list-to-array mapping needs to know how to generate an SQL literal for an array. @@ -52,5 +74,18 @@ protected override string GenerateNonNullSqlLiteral(object value) sb.Append("[]"); return sb.ToString(); } + + protected override void ConfigureParameter(DbParameter parameter) + { + base.ConfigureParameter(parameter); + + if (parameter is NpgsqlParameter npgsqlParameter) + { + if (NpgsqlDbType.HasValue) + npgsqlParameter.NpgsqlDbType = NpgsqlDbType.Value; + } + else + throw new InvalidOperationException($"Npgsql-specific type mapping {GetType().Name} being used with non-Npgsql parameter type {parameter.GetType().Name}"); + } } } diff --git a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs index e9fdc4aec..14f8ecf2b 100644 --- a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs +++ b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs @@ -349,8 +349,12 @@ protected virtual RelationalTypeMapping FindBaseMapping(in RelationalTypeMapping // we proceed with a CLR type lookup (if the type doesn't exist at all the failure will come later). } - if (clrType == null || !ClrTypeMappings.TryGetValue(clrType, out var mapping)) + if (clrType == null || + !ClrTypeMappings.TryGetValue(clrType, out var mapping) || + storeTypeName != null && mapping.StoreType != storeTypeName) + { return null; + } // If needed, clone the mapping with the configured length/precision/scale // TODO: Cache size/precision/scale mappings? @@ -395,43 +399,52 @@ protected virtual RelationalTypeMapping FindBaseMapping(in RelationalTypeMapping protected virtual RelationalTypeMapping FindArrayMapping(in RelationalTypeMappingInfo mappingInfo) { - // PostgreSQL array type names are the element plus [] + var clrType = mappingInfo.ClrType; + Type elementClrType = null; + + if (clrType != null && !clrType.TryGetElementType(out elementClrType)) + return null; // Not an array/list + var storeType = mappingInfo.StoreTypeName; var storeTypeNameBase = mappingInfo.StoreTypeNameBase; if (storeType != null) { + // PostgreSQL array type names are the element plus [] if (!storeType.EndsWith("[]")) return null; - // Construct a RelationalTypeMappingInfo for the element type and look that up - // Note that we scaffold PostgreSQL arrays to C# arrays, not lists (which are also supported) - // TODO: In theory support the multiple mappings just like we do with scalars above - // (e.g. DateTimeOffset[] vs. DateTime[] + var elementStoreType = storeType.Substring(0, storeType.Length - 2); + var elementStoreTypeNameBase = storeTypeNameBase.Substring(0, storeType.Length - 2); - var elementMapping = FindMapping(new RelationalTypeMappingInfo( - storeType.Substring(0, storeType.Length - 2), - storeTypeNameBase.Substring(0, storeTypeNameBase.Length - 2), - mappingInfo.IsUnicode, - mappingInfo.Size, - mappingInfo.Precision, - mappingInfo.Scale)); + RelationalTypeMapping elementMapping; - if (elementMapping != null) + if (elementClrType == null) { - var added = StoreTypeMappings.TryAdd(storeType, - new RelationalTypeMapping[] - { - new NpgsqlArrayArrayTypeMapping(storeType, elementMapping), - new NpgsqlArrayListTypeMapping(storeType, elementMapping) - }); - Debug.Assert(added); - var mapping = FindBaseMapping(mappingInfo); - Debug.Assert(mapping != null); - return mapping; + elementMapping = FindMapping(new RelationalTypeMappingInfo( + elementStoreType, elementStoreTypeNameBase, + mappingInfo.IsUnicode, mappingInfo.Size, mappingInfo.Precision, mappingInfo.Scale)); } + else + { + elementMapping = FindMapping(new RelationalTypeMappingInfo( + elementClrType, elementStoreType, elementStoreTypeNameBase, + mappingInfo.IsKeyOrIndex, mappingInfo.IsUnicode, mappingInfo.Size, mappingInfo.IsRowVersion, + mappingInfo.IsFixedLength, mappingInfo.Precision, mappingInfo.Scale)); + + // If an element mapping was found only with the help of a value converter, return null and EF will + // construct the corresponding array mapping with a value converter. + if (elementMapping?.Converter != null) + return null; + } + + // If no mapping was found for the element, there's no mapping for the array. + // Also, arrays of arrays aren't supported (as opposed to multidimensional arrays) by PostgreSQL + if (elementMapping == null || elementMapping is NpgsqlArrayTypeMapping) + return null; + + return new NpgsqlArrayArrayTypeMapping(storeType, elementMapping); } - var clrType = mappingInfo.ClrType; if (clrType == null) return null; diff --git a/test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs index 37bf61bbf..3c36244f9 100644 --- a/test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs +++ b/test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs @@ -1,4 +1,6 @@ using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; using System.Linq; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.TestUtilities; @@ -42,7 +44,7 @@ public void Index_with_constant() Assert.Single(actual); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE s.""SomeArray""[1] = 3"); } @@ -59,7 +61,7 @@ public void Index_with_non_constant() AssertSql( @"@__x_0='0' -SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" +SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE s.""SomeArray""[@__x_0 + 1] = 3"); } @@ -79,7 +81,7 @@ public void SequenceEqual_with_parameter() AssertSql( @"@__arr_0='System.Int32[]' (DbType = Object) -SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" +SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE s.""SomeArray"" = @__arr_0 LIMIT 2"); @@ -93,7 +95,7 @@ public void SequenceEqual_with_array_literal() Assert.Equal(new[] { 3, 4 }, x.SomeArray); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE s.""SomeArray"" = ARRAY[3,4]::integer[] LIMIT 2"); @@ -111,7 +113,7 @@ public void Contains_with_literal() Assert.Equal(new[] { 3, 4 }, x.SomeArray); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE 3 = ANY (s.""SomeArray"") LIMIT 2"); @@ -129,7 +131,7 @@ public void Contains_with_parameter() AssertSql( @"@__p_0='3' -SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" +SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE @__p_0 = ANY (s.""SomeArray"") LIMIT 2"); @@ -143,12 +145,30 @@ public void Contains_with_column() Assert.Equal(new[] { 3, 4 }, x.SomeArray); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE s.""Id"" + 2 = ANY (s.""SomeArray"") LIMIT 2"); } + [Fact] + public void Byte_array_parameter_contains_column() + { + using var ctx = CreateContext(); + var values = new byte[] { 20 }; + var x = ctx.SomeEntities.Single(e => values.Contains(e.SomeByte)); + + Assert.Equal(2, x.Id); + // Note: EF Core prints the parameter as a bytea, but it's actually a smallint[] (otherwise ANY would fail) + AssertSql( + @"@__values_0='0x14' (DbType = Object) + +SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" +FROM ""SomeEntities"" AS s +WHERE s.""SomeByte"" = ANY (@__values_0) +LIMIT 2"); + } + #endregion #region Length @@ -161,7 +181,7 @@ public void Length() Assert.Equal(new[] { 3, 4 }, x.SomeArray); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE cardinality(s.""SomeArray"") = 2 LIMIT 2"); @@ -175,7 +195,7 @@ public void Length_on_EF_Property() Assert.Equal(new[] { 3, 4 }, x.SomeArray); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE cardinality(s.""SomeArray"") = 2 LIMIT 2"); @@ -216,7 +236,7 @@ public void Any_like() .ToList(); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE s.""SomeText"" LIKE ANY (ARRAY['a%','b%','c%']::text[])"); } @@ -230,7 +250,7 @@ public void Any_ilike() .ToList(); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE s.""SomeText"" ILIKE ANY (ARRAY['a%','b%','c%']::text[])"); } @@ -276,10 +296,11 @@ public void Any_Contains() Assert.Empty(results); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE (ARRAY[2,3]::integer[] && s.""SomeArray"")", - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + // + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE (ARRAY[1,2]::integer[] && s.""SomeArray"")"); } @@ -300,11 +321,11 @@ public void All_Contains() Assert.Empty(results); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE (ARRAY[5,6]::integer[] <@ s.""SomeArray"")", // - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE (ARRAY[4,5,6]::integer[] <@ s.""SomeArray"")"); } @@ -321,7 +342,7 @@ public void Index_bytea_with_constant() Assert.Single(actual); AssertSql( - @"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" + @"SELECT s.""Id"", s.""SomeArray"", s.""SomeByte"", s.""SomeByteArray"", s.""SomeBytea"", s.""SomeMatrix"", s.""SomeText"" FROM ""SomeEntities"" AS s WHERE get_byte(s.""SomeBytea"", 0) = 3"); } @@ -365,16 +386,20 @@ public static void Seed(ArrayArrayQueryContext context) Id = 1, SomeArray = new[] { 3, 4 }, SomeBytea = new byte[] { 3, 4 }, + SomeByteArray = new byte[] { 3, 4 }, SomeMatrix = new[,] { { 5, 6 }, { 7, 8 } }, - SomeText = "foo" + SomeText = "foo", + SomeByte = 10 }, new SomeArrayEntity { Id = 2, SomeArray = new[] { 5, 6, 7 }, SomeBytea = new byte[] { 5, 6, 7 }, + SomeByteArray = new byte[] { 5, 6, 7 }, SomeMatrix = new[,] { { 10, 11 }, { 12, 13 } }, - SomeText = "bar" + SomeText = "bar", + SomeByte = 20 }); context.SaveChanges(); } @@ -386,7 +411,9 @@ public class SomeArrayEntity public int[] SomeArray { get; set; } public int[,] SomeMatrix { get; set; } public byte[] SomeBytea { get; set; } + public byte[] SomeByteArray { get; set; } public string SomeText { get; set; } + public byte SomeByte { get; set; } } public class ArrayArrayQueryFixture : SharedStoreFixtureBase diff --git a/test/EFCore.PG.FunctionalTests/Query/EnumQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/EnumQueryTest.cs index 26cdceec0..15ba09c40 100644 --- a/test/EFCore.PG.FunctionalTests/Query/EnumQueryTest.cs +++ b/test/EFCore.PG.FunctionalTests/Query/EnumQueryTest.cs @@ -65,7 +65,7 @@ public void Where_with_parameter() AssertSql( @"@__sad_0='Sad' (DbType = Object) -SELECT s.""Id"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedEnum"" +SELECT s.""Id"", s.""ByteEnum"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedByteEnum"", s.""UnmappedEnum"" FROM test.""SomeEntities"" AS s WHERE s.""MappedEnum"" = @__sad_0 LIMIT 2"); @@ -82,7 +82,7 @@ public void Where_with_unmapped_enum_parameter_downcasts_are_implicit() AssertSql( @"@__sad_0='1' -SELECT s.""Id"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedEnum"" +SELECT s.""Id"", s.""ByteEnum"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedByteEnum"", s.""UnmappedEnum"" FROM test.""SomeEntities"" AS s WHERE s.""UnmappedEnum"" = @__sad_0 LIMIT 2"); @@ -99,7 +99,7 @@ public void Where_with_unmapped_enum_parameter_downcasts_do_not_matter() AssertSql( @"@__sad_0='1' -SELECT s.""Id"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedEnum"" +SELECT s.""Id"", s.""ByteEnum"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedByteEnum"", s.""UnmappedEnum"" FROM test.""SomeEntities"" AS s WHERE s.""UnmappedEnum"" = @__sad_0 LIMIT 2"); @@ -116,7 +116,7 @@ public void Where_with_mapped_enum_parameter_downcasts_do_not_matter() AssertSql( @"@__sad_0='Sad' (DbType = Object) -SELECT s.""Id"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedEnum"" +SELECT s.""Id"", s.""ByteEnum"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedByteEnum"", s.""UnmappedEnum"" FROM test.""SomeEntities"" AS s WHERE s.""MappedEnum"" = @__sad_0 LIMIT 2"); @@ -130,12 +130,47 @@ public void Enum_ToString() var _ = ctx.SomeEntities.Single(e => e.MappedEnum.ToString().Contains("sa")); AssertSql( - @"SELECT s.""Id"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedEnum"" + @"SELECT s.""Id"", s.""ByteEnum"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedByteEnum"", s.""UnmappedEnum"" FROM test.""SomeEntities"" AS s WHERE STRPOS(CAST(s.""MappedEnum"" AS text), 'sa') > 0 LIMIT 2"); } + [Fact] + public void Where_byte_enum_array_contains_enum() + { + using var ctx = CreateContext(); + var values = new[] { ByteEnum.Sad }; + var result = ctx.SomeEntities.Single(e => values.Contains(e.ByteEnum)); + Assert.Equal(2, result.Id); + + AssertSql( + @"@__values_0='Npgsql.EntityFrameworkCore.PostgreSQL.Query.EnumQueryTest+ByteEnum[]' (DbType = Object) + +SELECT s.""Id"", s.""ByteEnum"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedByteEnum"", s.""UnmappedEnum"" +FROM test.""SomeEntities"" AS s +WHERE s.""ByteEnum"" = ANY (@__values_0) +LIMIT 2"); + } + + [Fact] + public void Where_unmapped_byte_enum_array_contains_enum() + { + using var ctx = CreateContext(); + var values = new[] { UnmappedByteEnum.Sad }; + var result = ctx.SomeEntities.Single(e => values.Contains(e.UnmappedByteEnum)); + Assert.Equal(2, result.Id); + + // Note: EF Core prints the parameter as a bytea, but it's actually a smallint[] (otherwise ANY would fail) + AssertSql( + @"@__values_0='0x01' (DbType = Object) + +SELECT s.""Id"", s.""ByteEnum"", s.""EnumValue"", s.""InferredEnum"", s.""MappedEnum"", s.""SchemaQualifiedEnum"", s.""UnmappedByteEnum"", s.""UnmappedEnum"" +FROM test.""SomeEntities"" AS s +WHERE s.""UnmappedByteEnum"" = ANY (@__values_0) +LIMIT 2"); + } + #endregion #region Support @@ -161,6 +196,7 @@ static EnumContext() { NpgsqlConnection.GlobalTypeMapper.MapEnum("test.mapped_enum"); NpgsqlConnection.GlobalTypeMapper.MapEnum("test.inferred_enum"); + NpgsqlConnection.GlobalTypeMapper.MapEnum("test.byte_enum"); NpgsqlConnection.GlobalTypeMapper.MapEnum("test.schema_qualified_enum"); } @@ -169,6 +205,7 @@ public EnumContext(DbContextOptions options) : base(options) {} protected override void OnModelCreating(ModelBuilder builder) => builder.HasPostgresEnum("mapped_enum", new[] { "happy", "sad" }) .HasPostgresEnum() + .HasPostgresEnum() .HasDefaultSchema("test") .HasPostgresEnum(); @@ -182,6 +219,8 @@ public static void Seed(EnumContext context) UnmappedEnum = UnmappedEnum.Happy, InferredEnum = InferredEnum.Happy, SchemaQualifiedEnum = SchemaQualifiedEnum.Happy, + ByteEnum = ByteEnum.Happy, + UnmappedByteEnum = UnmappedByteEnum.Happy, EnumValue = (int)MappedEnum.Happy }, new SomeEnumEntity @@ -191,6 +230,8 @@ public static void Seed(EnumContext context) UnmappedEnum = UnmappedEnum.Sad, InferredEnum = InferredEnum.Sad, SchemaQualifiedEnum = SchemaQualifiedEnum.Sad, + ByteEnum = ByteEnum.Sad, + UnmappedByteEnum = UnmappedByteEnum.Sad, EnumValue = (int)MappedEnum.Sad }); context.SaveChanges(); @@ -204,6 +245,8 @@ public class SomeEnumEntity public UnmappedEnum UnmappedEnum { get; set; } public InferredEnum InferredEnum { get; set; } public SchemaQualifiedEnum SchemaQualifiedEnum { get; set; } + public ByteEnum ByteEnum { get; set; } + public UnmappedByteEnum UnmappedByteEnum { get; set; } public int EnumValue { get; set; } } @@ -232,6 +275,18 @@ public enum SchemaQualifiedEnum Sad } + public enum ByteEnum : byte + { + Happy, + Sad + } + + public enum UnmappedByteEnum : byte + { + Happy, + Sad + } + // ReSharper disable once ClassNeverInstantiated.Global public class EnumFixture : SharedStoreFixtureBase { diff --git a/test/EFCore.PG.FunctionalTests/TestModels/Northwind/NorthwindNpgsqlContext.cs b/test/EFCore.PG.FunctionalTests/TestModels/Northwind/NorthwindNpgsqlContext.cs index 746a851b9..52b7b2052 100644 --- a/test/EFCore.PG.FunctionalTests/TestModels/Northwind/NorthwindNpgsqlContext.cs +++ b/test/EFCore.PG.FunctionalTests/TestModels/Northwind/NorthwindNpgsqlContext.cs @@ -13,35 +13,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) modelBuilder.HasPostgresExtension("uuid-ossp"); - modelBuilder.Entity() - .Property(c => c.CustomerID) - .HasColumnType("varchar(5)"); - - modelBuilder.Entity( - b => - { - b.Property(c => c.EmployeeID).HasColumnType("int4"); - b.Property(c => c.ReportsTo).HasColumnType("int4"); - }); - - modelBuilder.Entity() - .Property(o => o.EmployeeID) - .HasColumnType("int4"); - modelBuilder.Entity() - .Property(od => od.UnitPrice) - .HasColumnType("money"); - - modelBuilder.Entity( - b => - { - b.Property(p => p.UnitPrice).HasColumnType("money"); - b.Property(p => p.UnitsInStock).HasColumnType("int2"); - }); - - modelBuilder.Entity() - .Property(p => p.UnitPrice) - .HasColumnType("money"); - #pragma warning disable CS0618 // Type or member is obsolete modelBuilder.Query().HasNoKey().ToQuery( () => CustomerQueries.FromSqlInterpolated($@"SELECT ""c"".""CustomerID"" || {_empty} as ""CustomerID"", ""c"".""Address"", ""c"".""City"", ""c"".""CompanyName"", ""c"".""ContactName"", ""c"".""ContactTitle"", ""c"".""Country"", ""c"".""Fax"", ""c"".""Phone"", ""c"".""PostalCode"", ""c"".""Region"" FROM ""Customers"" AS ""c""" From 406ca719f95694f3debad6336bb813ff2bd398c0 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 14 Jan 2020 18:56:06 +0100 Subject: [PATCH 3/4] Update src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs Co-Authored-By: Yoh Deadfall --- src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs index 14f8ecf2b..3624086c4 100644 --- a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs +++ b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs @@ -351,7 +351,7 @@ protected virtual RelationalTypeMapping FindBaseMapping(in RelationalTypeMapping if (clrType == null || !ClrTypeMappings.TryGetValue(clrType, out var mapping) || - storeTypeName != null && mapping.StoreType != storeTypeName) + storeTypeName != null && storeTypeName != mapping.StoreType) { return null; } From df106a2e7a1d2910a5279adf0a04b5f46a7daccc Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 14 Jan 2020 18:59:35 +0100 Subject: [PATCH 4/4] Address review and fix some bugs --- .../NpgsqlNodaTimeTypeMappingSourcePlugin.cs | 2 +- .../Mapping/NpgsqlArrayTypeMapping.cs | 26 +++++++++---------- .../Internal/Mapping/NpgsqlTypeMapping.cs | 9 ++++--- .../Internal/NpgsqlTypeMappingSource.cs | 5 ++-- .../Storage/NpgsqlTypeMappingSourceTest.cs | 2 ++ 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/EFCore.PG.NodaTime/Storage/Internal/NpgsqlNodaTimeTypeMappingSourcePlugin.cs b/src/EFCore.PG.NodaTime/Storage/Internal/NpgsqlNodaTimeTypeMappingSourcePlugin.cs index 7535c7f93..b0a9e216b 100644 --- a/src/EFCore.PG.NodaTime/Storage/Internal/NpgsqlNodaTimeTypeMappingSourcePlugin.cs +++ b/src/EFCore.PG.NodaTime/Storage/Internal/NpgsqlNodaTimeTypeMappingSourcePlugin.cs @@ -155,7 +155,7 @@ protected virtual RelationalTypeMapping FindArrayMapping(in RelationalTypeMappin return null; var elementStoreType = storeType.Substring(0, storeType.Length - 2); - var elementStoreTypeNameBase = storeTypeNameBase.Substring(0, storeType.Length - 2); + var elementStoreTypeNameBase = storeTypeNameBase.Substring(0, storeTypeNameBase.Length - 2); RelationalTypeMapping elementMapping; diff --git a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayTypeMapping.cs b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayTypeMapping.cs index 11a573ce0..26d8da5fb 100644 --- a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayTypeMapping.cs +++ b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayTypeMapping.cs @@ -36,13 +36,12 @@ protected NpgsqlArrayTypeMapping(RelationalTypeMappingParameters parameters, Rel // If the element mapping has an NpgsqlDbType or DbType, set our own NpgsqlDbType as an array of that. // Otherwise let the ADO.NET layer infer the PostgreSQL type. We can't always let it infer, otherwise // when given a byte[] it will infer byte (but we want smallint[]) - var elementNpgsqlDbType = elementMapping is NpgsqlTypeMapping elementNpgsqlTypeMapping - ? elementNpgsqlTypeMapping.NpgsqlDbType - : elementMapping.DbType.HasValue - ? new NpgsqlParameter { DbType = elementMapping.DbType.Value }.NpgsqlDbType - : (NpgsqlDbType?)null; - if (elementNpgsqlDbType != null) - NpgsqlDbType = elementNpgsqlDbType | NpgsqlTypes.NpgsqlDbType.Array; + NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Array | + (elementMapping is NpgsqlTypeMapping elementNpgsqlTypeMapping + ? elementNpgsqlTypeMapping.NpgsqlDbType + : elementMapping.DbType.HasValue + ? new NpgsqlParameter { DbType = elementMapping.DbType.Value }.NpgsqlDbType + : default(NpgsqlDbType?)); } // The array-to-array mapping needs to know how to generate an SQL literal for a List<>, and @@ -77,15 +76,14 @@ protected override string GenerateNonNullSqlLiteral(object value) protected override void ConfigureParameter(DbParameter parameter) { + var npgsqlParameter = parameter as NpgsqlParameter; + if (npgsqlParameter == null) + throw new ArgumentException($"Npgsql-specific type mapping {GetType()} being used with non-Npgsql parameter type {parameter.GetType().Name}"); + base.ConfigureParameter(parameter); - if (parameter is NpgsqlParameter npgsqlParameter) - { - if (NpgsqlDbType.HasValue) - npgsqlParameter.NpgsqlDbType = NpgsqlDbType.Value; - } - else - throw new InvalidOperationException($"Npgsql-specific type mapping {GetType().Name} being used with non-Npgsql parameter type {parameter.GetType().Name}"); + if (NpgsqlDbType.HasValue) + npgsqlParameter.NpgsqlDbType = NpgsqlDbType.Value; } } } diff --git a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlTypeMapping.cs b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlTypeMapping.cs index 154352991..86510da43 100644 --- a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlTypeMapping.cs +++ b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlTypeMapping.cs @@ -39,12 +39,13 @@ protected NpgsqlTypeMapping(RelationalTypeMappingParameters parameters, NpgsqlDb protected override void ConfigureParameter(DbParameter parameter) { + var npgsqlParameter = parameter as NpgsqlParameter; + if (npgsqlParameter == null) + throw new ArgumentException($"Npgsql-specific type mapping {GetType()} being used with non-Npgsql parameter type {parameter.GetType().Name}"); + base.ConfigureParameter(parameter); - if (parameter is NpgsqlParameter npgsqlParameter) - npgsqlParameter.NpgsqlDbType = NpgsqlDbType; - else - throw new InvalidOperationException($"Npgsql-specific type mapping {GetType().Name} being used with non-Npgsql parameter type {parameter.GetType().Name}"); + npgsqlParameter.NpgsqlDbType = NpgsqlDbType; } } } diff --git a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs index 3624086c4..da13bc38b 100644 --- a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs +++ b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs @@ -351,7 +351,8 @@ protected virtual RelationalTypeMapping FindBaseMapping(in RelationalTypeMapping if (clrType == null || !ClrTypeMappings.TryGetValue(clrType, out var mapping) || - storeTypeName != null && storeTypeName != mapping.StoreType) + // Special case for byte[] mapped as smallint[] - don't return bytea mapping + storeTypeName != null && storeTypeName == "smallint[]") { return null; } @@ -414,7 +415,7 @@ protected virtual RelationalTypeMapping FindArrayMapping(in RelationalTypeMappin return null; var elementStoreType = storeType.Substring(0, storeType.Length - 2); - var elementStoreTypeNameBase = storeTypeNameBase.Substring(0, storeType.Length - 2); + var elementStoreTypeNameBase = storeTypeNameBase.Substring(0, storeTypeNameBase.Length - 2); RelationalTypeMapping elementMapping; diff --git a/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs b/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs index 0a7f24ffb..e13e06ced 100644 --- a/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs +++ b/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs @@ -79,6 +79,7 @@ public void Timestamp_without_time_zone_Array_5() [Theory] [InlineData(typeof(int), "integer")] [InlineData(typeof(int[]), "integer[]")] + [InlineData(typeof(byte[]), "bytea")] [InlineData(typeof(DummyType), "dummy")] [InlineData(typeof(NpgsqlRange), "int4range")] [InlineData(typeof(NpgsqlRange), "floatrange")] @@ -91,6 +92,7 @@ public void By_ClrType(Type clrType, string expectedStoreType) [Theory] [InlineData("integer", typeof(int))] [InlineData("integer[]", typeof(int[]))] + [InlineData("smallint[]", typeof(byte[]))] [InlineData("dummy", typeof(DummyType))] [InlineData("int4range", typeof(NpgsqlRange))] [InlineData("floatrange", typeof(NpgsqlRange))]