diff --git a/Dapper/SqlMapper.cs b/Dapper/SqlMapper.cs index 8f3956cab..f613a364e 100644 --- a/Dapper/SqlMapper.cs +++ b/Dapper/SqlMapper.cs @@ -3472,7 +3472,7 @@ private static void GenerateValueTupleDeserializer(Type valueTupleType, DbDataRe il, startBound + i, ref stringEnumLocal, - valueCopyLocal: null, + valueCopyLocals: null, reader.GetFieldType(startBound + i), targetType, out var isDbNullLabel, out bool popWhenNull); @@ -3517,6 +3517,7 @@ private static void GenerateDeserializerFromMap(Type type, DbDataReader reader, var returnValueLocal = il.DeclareLocal(type); il.Emit(OpCodes.Ldc_I4_0); il.Emit(OpCodes.Stloc, currentIndexDiagnosticLocal); + var valueCopyDiagnosticLocals = new ValueCopyDiagnosticLocals(il); var names = Enumerable.Range(startBound, length).Select(i => reader.GetName(i)).ToArray(); @@ -3610,7 +3611,6 @@ private static void GenerateDeserializerFromMap(Type type, DbDataReader reader, bool first = true; var allDone = il.DefineLabel(); var stringEnumLocal = (LocalBuilder?)null; - var valueCopyDiagnosticLocal = il.DeclareLocal(typeof(object)); bool applyNullSetting = Settings.ApplyNullValues; foreach (var item in members) { @@ -3625,7 +3625,7 @@ private static void GenerateDeserializerFromMap(Type type, DbDataReader reader, EmitInt32(il, index); il.Emit(OpCodes.Stloc, currentIndexDiagnosticLocal); - LoadReaderValueOrBranchToDBNullLabel(il, index, ref stringEnumLocal, valueCopyDiagnosticLocal, reader.GetFieldType(index), memberType, out var isDbNullLabel, out bool popWhenNull); + LoadReaderValueOrBranchToDBNullLabel(il, index, ref stringEnumLocal, valueCopyDiagnosticLocals, reader.GetFieldType(index), memberType, out var isDbNullLabel, out bool popWhenNull); if (specializedConstructor is null) { @@ -3711,9 +3711,10 @@ private static void GenerateDeserializerFromMap(Type type, DbDataReader reader, } il.MarkLabel(allDone); il.BeginCatchBlock(typeof(Exception)); // stack is Exception + valueCopyDiagnosticLocals.EmitBoxTypedValueOfCurrentColumn(il, currentIndexDiagnosticLocal); // stack is Exception il.Emit(OpCodes.Ldloc, currentIndexDiagnosticLocal); // stack is Exception, index il.Emit(OpCodes.Ldarg_0); // stack is Exception, index, reader - il.Emit(OpCodes.Ldloc, valueCopyDiagnosticLocal); // stack is Exception, index, reader, value + il.Emit(OpCodes.Ldloc, valueCopyDiagnosticLocals.ObjectLocal); // stack is Exception, index, reader, value il.EmitCall(OpCodes.Call, typeof(SqlMapper).GetMethod(nameof(SqlMapper.ThrowDataException))!, null); il.EndExceptionBlock(); @@ -3740,7 +3741,75 @@ private static void LoadDefaultValue(ILGenerator il, Type type) } } - private static void LoadReaderValueViaGetFieldValue(ILGenerator il, int index, Type memberType, LocalBuilder? valueCopyLocal, Label isDbNullLabel, out bool popWhenNull) + /// + /// The locals that keep the value of the column being read, so that can show it + /// when a column fails. A value read via is already an object and goes into + /// . A value read via stays unboxed in a + /// local of its own type and is boxed in the catch block, only if its column is the one that failed, so that the + /// GetFieldValue path does not allocate per value. + /// + private sealed class ValueCopyDiagnosticLocals + { + public LocalBuilder ObjectLocal { get; } + private readonly LocalBuilder typedValueIndexLocal; // the column whose value is in a typed local, or -1 + private readonly Dictionary typedLocals = []; + private readonly List> typedColumns = []; + + public ValueCopyDiagnosticLocals(ILGenerator il) + { + ObjectLocal = il.DeclareLocal(typeof(object)); + typedValueIndexLocal = il.DeclareLocal(typeof(int)); + EmitInt32(il, -1); + il.Emit(OpCodes.Stloc, typedValueIndexLocal); + } + + /// + /// Stores the value on top of the stack in the typed local for , leaving it on the stack. + /// + public void EmitStoreTypedValue(ILGenerator il, int index, Type type) + { + if (!typedLocals.TryGetValue(type, out var local)) + { + local = il.DeclareLocal(type); + typedLocals.Add(type, local); + } + il.Emit(OpCodes.Dup); // stack is now [...][T][T] + il.Emit(OpCodes.Stloc, local); // stack is now [...][T] + EmitInt32(il, index); + il.Emit(OpCodes.Stloc, typedValueIndexLocal); + typedColumns.Add(new KeyValuePair(index, type)); + } + + /// + /// For the catch block: if the value of the failing column is in a typed local, boxes it into + /// . Leaves the stack as it was. + /// + public void EmitBoxTypedValueOfCurrentColumn(ILGenerator il, LocalBuilder currentIndexLocal) + { + if (typedColumns.Count == 0) return; + + var done = il.DefineLabel(); + // a typed value belongs to the failing column only if it was stored while reading that column + il.Emit(OpCodes.Ldloc, typedValueIndexLocal); + il.Emit(OpCodes.Ldloc, currentIndexLocal); + il.Emit(OpCodes.Bne_Un, done); + foreach (var column in typedColumns) + { + var next = il.DefineLabel(); + il.Emit(OpCodes.Ldloc, typedValueIndexLocal); + EmitInt32(il, column.Key); + il.Emit(OpCodes.Bne_Un, next); + il.Emit(OpCodes.Ldloc, typedLocals[column.Value]); + il.Emit(OpCodes.Box, column.Value); + il.Emit(OpCodes.Stloc, ObjectLocal); + il.Emit(OpCodes.Br, done); + il.MarkLabel(next); + } + il.MarkLabel(done); + } + } + + private static void LoadReaderValueViaGetFieldValue(ILGenerator il, int index, Type memberType, ValueCopyDiagnosticLocals? valueCopyLocals, Label isDbNullLabel, out bool popWhenNull) { popWhenNull = false; var underlyingType = Nullable.GetUnderlyingType(memberType) ?? memberType; @@ -3755,14 +3824,18 @@ private static void LoadReaderValueViaGetFieldValue(ILGenerator il, int index, T il.Emit(OpCodes.Ldarg_0); // stack is now [...][reader] EmitInt32(il, index); // stack is now [...][reader][index] il.Emit(OpCodes.Callvirt, getFieldValueT.MakeGenericMethod(underlyingType)); // stack is now [...][T] - if (valueCopyLocal is not null) + if (valueCopyLocals is not null) { - il.Emit(OpCodes.Dup); // stack is now [...][T][T] if (underlyingType.IsValueType) { - il.Emit(OpCodes.Box, underlyingType); // stack is now [...][T][value-as-object] + // keep the value unboxed; the catch block boxes it only if this column fails + valueCopyLocals.EmitStoreTypedValue(il, index, underlyingType); // stack is now [...][T] + } + else + { + il.Emit(OpCodes.Dup); // stack is now [...][T][T] + il.Emit(OpCodes.Stloc, valueCopyLocals.ObjectLocal); // stack is now [...][T] } - il.Emit(OpCodes.Stloc, valueCopyLocal); // stack is now [...][T] } if (underlyingType != memberType) { @@ -3771,12 +3844,12 @@ private static void LoadReaderValueViaGetFieldValue(ILGenerator il, int index, T } } - private static void LoadReaderValueOrBranchToDBNullLabel(ILGenerator il, int index, ref LocalBuilder? stringEnumLocal, LocalBuilder? valueCopyLocal, Type colType, Type memberType, out Label isDbNullLabel, out bool popWhenNull) + private static void LoadReaderValueOrBranchToDBNullLabel(ILGenerator il, int index, ref LocalBuilder? stringEnumLocal, ValueCopyDiagnosticLocals? valueCopyLocals, Type colType, Type memberType, out Label isDbNullLabel, out bool popWhenNull) { isDbNullLabel = il.DefineLabel(); if (UseGetFieldValue(memberType) && !IsDateTimeFamilyConversion(colType, Nullable.GetUnderlyingType(memberType) ?? memberType)) { - LoadReaderValueViaGetFieldValue(il, index, memberType, valueCopyLocal, isDbNullLabel, out popWhenNull); + LoadReaderValueViaGetFieldValue(il, index, memberType, valueCopyLocals, isDbNullLabel, out popWhenNull); return; } @@ -3786,10 +3859,10 @@ private static void LoadReaderValueOrBranchToDBNullLabel(ILGenerator il, int ind // default impl: use GetValue il.Emit(OpCodes.Callvirt, getItem); // stack is now [...][value-as-object] - if (valueCopyLocal is not null) + if (valueCopyLocals is not null) { il.Emit(OpCodes.Dup); // stack is now [...][value-as-object][value-as-object] - il.Emit(OpCodes.Stloc, valueCopyLocal); // stack is now [...][value-as-object] + il.Emit(OpCodes.Stloc, valueCopyLocals.ObjectLocal); // stack is now [...][value-as-object] } if (memberType == typeof(char) || memberType == typeof(char?)) diff --git a/tests/Dapper.Tests/DataReaderTests.cs b/tests/Dapper.Tests/DataReaderTests.cs index f98bfabed..bd36e8c1d 100644 --- a/tests/Dapper.Tests/DataReaderTests.cs +++ b/tests/Dapper.Tests/DataReaderTests.cs @@ -1,4 +1,6 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; +using System.Data; using System.Data.Common; using System.Linq; using Xunit; @@ -283,6 +285,72 @@ union all Assert.Equal("qwe", bar.HazNameIdObject.Name); } + [Fact] + public void GetFieldValue_ErrorMessageShowsTheValue() + { + // the message names the value that was read whichever reader method read it; on the + // GetFieldValue path the value is kept unboxed and boxed only when its column fails + SqlMapper.AddTypeMap(typeof(long), DbType.Int64, useGetFieldValue: true); + SqlMapper.AddTypeMap(typeof(int), DbType.Int32, useGetFieldValue: true); + try + { + var ex = Assert.Throws(() => connection.Query("select cast(7 as bigint) as Id, 42 as Value -- GetFieldValue")); + Assert.Equal("Error parsing column 1 (Value=42 - Int32)", ex.Message); + Assert.IsType(ex.InnerException); + + ex = Assert.Throws(() => connection.Query("select cast(7 as bigint) as Id, 42 as Value -- GetFieldValue")); + Assert.Equal("Error parsing column 1 (Value=42 - Int32)", ex.Message); + Assert.IsType(ex.InnerException); + } + finally + { + SqlMapper.AddTypeMap(typeof(long), DbType.Int64, useGetFieldValue: false); + SqlMapper.AddTypeMap(typeof(int), DbType.Int32, useGetFieldValue: false); + } + + // the same message as the GetValue path gives; a separate type, because deserializers are cached + // per type and column shape, so the same type would reuse the deserializer compiled above + var viaGetValue = Assert.Throws(() => connection.Query("select cast(7 as bigint) as Id, 42 as Value -- GetValue")); + Assert.Equal("Error parsing column 1 (Value=42 - Int32)", viaGetValue.Message); + } + + [Fact] + public void GetFieldValue_ErrorMessageWhenTheReadItselfFails() + { + // when GetFieldValue throws there is no value to show, and the message must not show the previous column's + SqlMapper.AddTypeMap(typeof(long), DbType.Int64, useGetFieldValue: true); + SqlMapper.AddTypeMap(typeof(int), DbType.Int32, useGetFieldValue: true); + try + { + var ex = Assert.Throws(() => connection.Query("select cast(7 as bigint) as Id, 'abc' as Value -- GetFieldValue")); + Assert.StartsWith("Error parsing column 1 (Value=n/a - ", ex.Message); + Assert.NotNull(ex.InnerException); + } + finally + { + SqlMapper.AddTypeMap(typeof(long), DbType.Int64, useGetFieldValue: false); + SqlMapper.AddTypeMap(typeof(int), DbType.Int32, useGetFieldValue: false); + } + } + + private class HazThrowingSetter + { + public long Id { get; set; } + public int Value { get => 0; set => throw new InvalidOperationException("Value cannot be set"); } + } + + private class HazThrowingNullableSetter + { + public long Id { get; set; } + public int? Value { get => null; set => throw new InvalidOperationException("Value cannot be set"); } + } + + private class HazThrowingSetterViaGetValue + { + public long Id { get; set; } + public int Value { get => 0; set => throw new InvalidOperationException("Value cannot be set"); } + } + private abstract class Discriminated_BaseType { public abstract int Type { get; }