Fix type mappings for "text", "ntext", and "image"

Fixes #15953

src/EFCore.SqlServer/Storage/Internal/SqlServerByteArrayTypeMapping.cs

@@ -2,10 +2,12 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Data;
 using System.Data.Common;
 using System.Globalization;
 using System.Text;
 using JetBrains.Annotations;
+using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore.ChangeTracking;
 using Microsoft.EntityFrameworkCore.Storage;
 
@@ -21,6 +23,8 @@ public class SqlServerByteArrayTypeMapping : ByteArrayTypeMapping
     {
         private const int MaxSize = 8000;
 
+        private readonly SqlDbType? _sqlDbType;
+
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
         ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -32,15 +36,17 @@ public SqlServerByteArrayTypeMapping(
             int? size = null,
             bool fixedLength = false,
             ValueComparer comparer = null,
+            SqlDbType? sqlDbType = null,
             StoreTypePostfix? storeTypePostfix = null)
-            : base(
+            : this(
                 new RelationalTypeMappingParameters(
                     new CoreTypeMappingParameters(typeof(byte[]), null, comparer),
                     storeType ?? (fixedLength ? "binary" : "varbinary"),
                     storeTypePostfix ?? StoreTypePostfix.Size,
                     System.Data.DbType.Binary,
                     size: size,
-                    fixedLength: fixedLength))
+                    fixedLength: fixedLength),
+                sqlDbType)
         {
         }
 
@@ -50,9 +56,10 @@ public SqlServerByteArrayTypeMapping(
         ///     any release. You should only use it directly in your code with extreme caution and knowing that
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
-        protected SqlServerByteArrayTypeMapping(RelationalTypeMappingParameters parameters)
+        protected SqlServerByteArrayTypeMapping(RelationalTypeMappingParameters parameters, SqlDbType? sqlDbType)
             : base(parameters)
         {
+            _sqlDbType = sqlDbType;
         }
 
         private static int CalculateSize(int? size)
@@ -64,7 +71,7 @@ private static int CalculateSize(int? size)
         /// <param name="parameters"> The parameters for this mapping. </param>
         /// <returns> The newly created mapping. </returns>
         protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
-            => new SqlServerByteArrayTypeMapping(parameters);
+            => new SqlServerByteArrayTypeMapping(parameters, _sqlDbType);
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -83,6 +90,12 @@ protected override void ConfigureParameter(DbParameter parameter)
             var length = (value as byte[])?.Length;
             var maxSpecificSize = CalculateSize(Size);
 
+            if (_sqlDbType.HasValue
+                && parameter is SqlParameter sqlParameter) // To avoid crashing wrapping providers
+            {
+                sqlParameter.SqlDbType = _sqlDbType.Value;
+            }
+
             parameter.Size = value == null || value == DBNull.Value || length != null && length <= maxSpecificSize
                 ? maxSpecificSize
                 : -1;

src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs

@@ -5,6 +5,7 @@
 using System.Data;
 using System.Data.Common;
 using JetBrains.Annotations;
+using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore.Storage;
 
 namespace Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal
@@ -20,6 +21,7 @@ public class SqlServerStringTypeMapping : StringTypeMapping
         private const int UnicodeMax = 4000;
         private const int AnsiMax = 8000;
 
+        private readonly SqlDbType? _sqlDbType;
         private readonly int _maxSpecificSize;
 
         /// <summary>
@@ -33,6 +35,7 @@ public SqlServerStringTypeMapping(
             bool unicode = false,
             int? size = null,
             bool fixedLength = false,
+            SqlDbType? sqlDbType = null,
             StoreTypePostfix? storeTypePostfix = null)
             : this(
                 new RelationalTypeMappingParameters(
@@ -42,7 +45,8 @@ public SqlServerStringTypeMapping(
                     GetDbType(unicode, fixedLength),
                     unicode,
                     size,
-                    fixedLength))
+                    fixedLength),
+                sqlDbType)
         {
         }
 
@@ -62,10 +66,11 @@ private static string GetStoreName(bool unicode, bool fixedLength) => unicode
         ///     any release. You should only use it directly in your code with extreme caution and knowing that
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
-        protected SqlServerStringTypeMapping(RelationalTypeMappingParameters parameters)
+        protected SqlServerStringTypeMapping(RelationalTypeMappingParameters parameters, SqlDbType? sqlDbType)
             : base(parameters)
         {
             _maxSpecificSize = CalculateSize(parameters.Unicode, parameters.Size);
+            _sqlDbType = sqlDbType;
         }
 
         private static int CalculateSize(bool unicode, int? size)
@@ -83,7 +88,7 @@ private static int CalculateSize(bool unicode, int? size)
         /// <param name="parameters"> The parameters for this mapping. </param>
         /// <returns> The newly created mapping. </returns>
         protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
-            => new SqlServerStringTypeMapping(parameters);
+            => new SqlServerStringTypeMapping(parameters, _sqlDbType);
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -101,6 +106,12 @@ protected override void ConfigureParameter(DbParameter parameter)
             var value = parameter.Value;
             var length = (value as string)?.Length;
 
+            if (_sqlDbType.HasValue
+                && parameter is SqlParameter sqlParameter) // To avoid crashing wrapping providers
+            {
+                sqlParameter.SqlDbType = _sqlDbType.Value;
+            }
+
             parameter.Size = value == null || value == DBNull.Value || length != null && length <= _maxSpecificSize
                 ? _maxSpecificSize
                 : -1;

src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs

@@ -64,6 +64,9 @@ private readonly BoolTypeMapping _bool
         private readonly SqlServerStringTypeMapping _fixedLengthUnicodeString
             = new SqlServerStringTypeMapping(unicode: true, fixedLength: true);
 
+        private readonly SqlServerStringTypeMapping _textUnicodeString
+            = new SqlServerStringTypeMapping("ntext", unicode: true, sqlDbType: SqlDbType.NText);
+
         private readonly SqlServerStringTypeMapping _variableLengthUnicodeString
             = new SqlServerStringTypeMapping(unicode: true);
 
@@ -73,6 +76,9 @@ private readonly SqlServerStringTypeMapping _variableLengthMaxUnicodeString
         private readonly SqlServerStringTypeMapping _fixedLengthAnsiString
             = new SqlServerStringTypeMapping(fixedLength: true);
 
+        private readonly SqlServerStringTypeMapping _textAnsiString
+            = new SqlServerStringTypeMapping("text", sqlDbType: SqlDbType.Text);
+
         private readonly SqlServerStringTypeMapping _variableLengthAnsiString
             = new SqlServerStringTypeMapping();
 
@@ -82,6 +88,9 @@ private readonly SqlServerStringTypeMapping _variableLengthMaxAnsiString
         private readonly SqlServerByteArrayTypeMapping _variableLengthBinary
             = new SqlServerByteArrayTypeMapping();
 
+        private readonly SqlServerByteArrayTypeMapping _imageBinary
+            = new SqlServerByteArrayTypeMapping("image", sqlDbType: SqlDbType.Image);
+
         private readonly SqlServerByteArrayTypeMapping _variableLengthMaxBinary
             = new SqlServerByteArrayTypeMapping("varbinary(max)", storeTypePostfix: StoreTypePostfix.None);
 
@@ -197,14 +206,14 @@ public SqlServerTypeMappingSource(
                     { "decimal", _decimal },
                     { "double precision", _double },
                     { "float", _double },
-                    { "image", _variableLengthBinary },
+                    { "image", _imageBinary },
                     { "int", _int },
                     { "money", _money },
                     { "national char varying", _variableLengthUnicodeString },
                     { "national character varying", _variableLengthUnicodeString },
                     { "national character", _fixedLengthUnicodeString },
                     { "nchar", _fixedLengthUnicodeString },
-                    { "ntext", _variableLengthUnicodeString },
+                    { "ntext", _textUnicodeString },
                     { "numeric", _decimal },
                     { "nvarchar", _variableLengthUnicodeString },
                     { "nvarchar(max)", _variableLengthMaxUnicodeString },
@@ -214,7 +223,7 @@ public SqlServerTypeMappingSource(
                     { "smallint", _short },
                     { "smallmoney", _money },
                     { "sql_variant", _sqlVariant },
-                    { "text", _variableLengthAnsiString },
+                    { "text", _textAnsiString },
                     { "time", _time },
                     { "timestamp", _rowversion },
                     { "tinyint", _byte },

test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs

@@ -145,7 +145,7 @@ public void Plugins_work()
                 scaffoldedModel.ContextFile.Code);
         }
 
-        [Fact]
+        [ConditionalFact]
         public void Comments_use_fluent_api()
         {
             Test(
@@ -166,7 +166,7 @@ public void Comments_use_fluent_api()
                     model.FindEntityType("TestNamespace.Entity").GetProperty("Property").GetComment()));
         }
 
-        [Fact]
+        [ConditionalFact]
         public void Entity_comments_use_fluent_api()
         {
             Test(

test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs

@@ -1801,7 +1801,7 @@ public void Unmapped_column_is_ignored()
             Assert.Equal(1, columns.Count);
         }
 
-        [Fact]
+        [ConditionalFact]
         public void Column_and_table_comments()
         {
             var database = new DatabaseModel

test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Data;
 using System.Data.Common;
+using System.Linq;
 using System.Reflection;
 using Microsoft.EntityFrameworkCore.ChangeTracking;
 using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
@@ -93,9 +94,16 @@ public virtual void Create_and_clone_with_converter(Type mappingType, Type clrTy
             Assert.Equal(StoreTypePostfix.PrecisionAndScale, clone.StoreTypePostfix);
         }
 
-        [ConditionalTheory]
-        [InlineData(typeof(ByteArrayTypeMapping), typeof(byte[]))]
-        public virtual void Create_and_clone_sized_mappings_with_converter(Type mappingType, Type clrType)
+        [ConditionalFact]
+        public virtual void Create_and_clone_sized_mappings_with_converter()
+        {
+            ConversionCloneTest(typeof(ByteArrayTypeMapping), typeof(byte[]));
+        }
+
+        protected virtual void ConversionCloneTest(
+            Type mappingType,
+            Type clrType,
+            params object[] additionalArgs)
         {
             var mapping = (RelationalTypeMapping)Activator.CreateInstance(
                 mappingType,
@@ -108,7 +116,7 @@ public virtual void Create_and_clone_sized_mappings_with_converter(Type mappingT
                         size: 33,
                         fixedLength: true,
                         storeTypePostfix: StoreTypePostfix.Size)
-                },
+                }.Concat(additionalArgs).ToArray(),
                 null,
                 null);
 
@@ -149,9 +157,16 @@ public virtual void Create_and_clone_sized_mappings_with_converter(Type mappingT
             Assert.Equal(StoreTypePostfix.Size, clone.StoreTypePostfix);
         }
 
-        [ConditionalTheory]
-        [InlineData(typeof(StringTypeMapping), typeof(string))]
-        public virtual void Create_and_clone_unicode_sized_mappings_with_converter(Type mappingType, Type clrType)
+        [ConditionalFact]
+        public virtual void Create_and_clone_unicode_sized_mappings_with_converter()
+        {
+            UnicodeConversionCloneTest(typeof(StringTypeMapping), typeof(string));
+        }
+
+        protected virtual void UnicodeConversionCloneTest(
+            Type mappingType,
+            Type clrType,
+            params object[] additionalArgs)
         {
             var mapping = (RelationalTypeMapping)Activator.CreateInstance(
                 mappingType,
@@ -165,7 +180,7 @@ public virtual void Create_and_clone_unicode_sized_mappings_with_converter(Type
                         unicode: false,
                         fixedLength: true,
                         storeTypePostfix: StoreTypePostfix.Size)
-                },
+                }.Concat(additionalArgs).ToArray(),
                 null,
                 null);
 

test/EFCore.Specification.Tests/CustomConvertersTestBase.cs

@@ -234,7 +234,7 @@ protected class Email
             public static implicit operator string(Email email) => email._value;
         }
 
-        [ConditionalFact(Skip = "Issue #14935. Cannot eval 'where [e].Fuel.Equals(value(Microsoft.EntityFrameworkCore.CustomConvertersTestBase`1+Fuel[Microsoft.EntityFrameworkCore.CustomConvertersSqliteTest+CustomConvertersSqliteFixture]))'")]
+        [ConditionalFact]
         public virtual void Can_query_and_update_with_conversion_for_custom_struct()
         {
             using (var context = CreateContext())

test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs

@@ -6,6 +6,7 @@
 using System.ComponentModel.DataAnnotations.Schema;
 using System.Linq;
 using System.Text;
+using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Metadata;
@@ -569,12 +570,23 @@ public virtual void Can_query_using_any_mapped_data_types_with_nulls()
                     entity,
                     context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.StringAsNationalCharacterVaryingMax == param29));
 
-                // TODO: See issue#15953
-                //string param30 = null;
-                //Assert.Same(entity, context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.StringAsText == param30));
+                string param30 = null;
+                var message30 = Assert.Throws<SqlException>(
+                    () => context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.StringAsText == param30)).Message;
 
-                //string param31 = null;
-                //Assert.Same(entity, context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.StringAsNtext == param31));
+                // Expect: "The data types text and text are incompatible in the equal to operator."
+                Assert.NotEqual(
+                    message30.IndexOf("text", StringComparison.Ordinal),
+                    message30.LastIndexOf("text", StringComparison.Ordinal));
+
+                string param31 = null;
+                var message31 = Assert.Throws<SqlException>(
+                    () => context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.StringAsNtext == param31)).Message;
+
+                // Expect: "The data types ntext and ntext are incompatible in the equal to operator."
+                Assert.NotEqual(
+                    message31.IndexOf("ntext", StringComparison.Ordinal),
+                    message31.LastIndexOf("ntext", StringComparison.Ordinal));
 
                 byte[] param35 = null;
                 Assert.Same(entity, context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.BytesAsVarbinaryMax == param35));
@@ -583,8 +595,14 @@ public virtual void Can_query_using_any_mapped_data_types_with_nulls()
                 Assert.Same(
                     entity, context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.BytesAsBinaryVaryingMax == param36));
 
-                //byte[] param37 = null;
-                //Assert.Same(entity, context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.BytesAsImage == param37));
+                byte[] param37 = null;
+                var message37 = Assert.Throws<SqlException>(
+                    () => context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.BytesAsImage == param37)).Message;
+
+                // Expect: "The data types image and image are incompatible in the equal to operator."
+                Assert.NotEqual(
+                    message37.IndexOf("image", StringComparison.Ordinal),
+                    message37.LastIndexOf("image", StringComparison.Ordinal));
 
                 decimal? param38 = null;
                 Assert.Same(entity, context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.Decimal == param38));

test/EFCore.SqlServer.Tests/Storage/SqlServerTypeMappingTest.cs

@@ -105,16 +105,22 @@ public override void Create_and_clone_with_converter(Type mappingType, Type clrT
             base.Create_and_clone_with_converter(mappingType, clrType);
         }
 
-        [InlineData(typeof(SqlServerByteArrayTypeMapping), typeof(byte[]))]
-        public override void Create_and_clone_sized_mappings_with_converter(Type mappingType, Type clrType)
+        [ConditionalFact]
+        public virtual void Create_and_clone_SQL_Server_sized_mappings_with_converter()
         {
-            base.Create_and_clone_sized_mappings_with_converter(mappingType, clrType);
+            ConversionCloneTest(
+                typeof(SqlServerByteArrayTypeMapping),
+                typeof(byte[]),
+                SqlDbType.Image);
         }
 
-        [InlineData(typeof(SqlServerStringTypeMapping), typeof(string))]
-        public override void Create_and_clone_unicode_sized_mappings_with_converter(Type mappingType, Type clrType)
+        [ConditionalFact]
+        public virtual void Create_and_clone_SQL_Server_unicode_sized_mappings_with_converter()
         {
-            base.Create_and_clone_unicode_sized_mappings_with_converter(mappingType, clrType);
+            UnicodeConversionCloneTest(
+                typeof(SqlServerStringTypeMapping),
+                typeof(string),
+                SqlDbType.Text);
         }
 
         [ConditionalFact]