Fix mapping issues around arrays and byte arrays

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;
+        }
+    }
+}

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, storeTypeNameBase.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,32 +194,33 @@ 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;
 
                 // Arrays of arrays aren't supported (as opposed to multidimensional arrays) by PostgreSQL
                 if (elementMapping is NpgsqlArrayTypeMapping)
                     return null;
 
-                return ClrTypeMappings.GetOrAdd(clrType, new NpgsqlArrayListTypeMapping(elementMapping, clrType));
+                return new NpgsqlArrayListTypeMapping(elementMapping, clrType);
             }
 
             return null;

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;
+        }
     }
 }

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<T> 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<T> 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)))));
             }
 

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),

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.
     /// </summary>
     /// <remarks>
-    /// Note that mapping PostgreSQL arrays to .NET List{T} is also supported via <see cref="NpgsqlArrayListTypeMapping"/>.
-    /// See: https://www.postgresql.org/docs/current/static/arrays.html
+    /// <para>
+    /// Note that mapping PostgreSQL arrays to .NET <see cref="List{T}"/> is also supported via
+    /// <see cref="NpgsqlArrayListTypeMapping"/>.
+    /// </para>
+    ///
+    /// <para>See: https://www.postgresql.org/docs/current/static/arrays.html</para>
     /// </remarks>
     public class NpgsqlArrayArrayTypeMapping : NpgsqlArrayTypeMapping
     {

src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayListTypeMapping.cs

@@ -11,8 +11,11 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping
     /// Maps PostgreSQL arrays to <see cref="List{T}"/>.
     /// </summary>
     /// <remarks>
+    /// <para>
     /// Note that mapping PostgreSQL arrays to .NET arrays is also supported via <see cref="NpgsqlArrayArrayTypeMapping"/>.
-    /// See: https://www.postgresql.org/docs/current/static/arrays.html
+    /// </para>
+    ///
+    /// <para>See: https://www.postgresql.org/docs/current/static/arrays.html</para>
     /// </remarks>
     public class NpgsqlArrayListTypeMapping : NpgsqlArrayTypeMapping
     {

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,26 @@ public abstract class NpgsqlArrayTypeMapping : RelationalTypeMapping
         /// </summary>
         public RelationalTypeMapping ElementMapping { get; }
 
+        /// <summary>
+        /// The database type used by Npgsql.
+        /// </summary>
+        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[])
+            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
         // the list-to-array mapping needs to know how to generate an SQL literal for an array.
@@ -52,5 +73,17 @@ protected override string GenerateNonNullSqlLiteral(object value)
             sb.Append("[]");
             return sb.ToString();
         }
+
+        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 (NpgsqlDbType.HasValue)
+                npgsqlParameter.NpgsqlDbType = NpgsqlDbType.Value;
+        }
     }
 }

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;
         }
     }
 }