Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

IDataReader.GetRowParser<string>() throws exception #1822

Open
p21x opened this issue Sep 6, 2022 · 2 comments · May be fixed by #1824
Open

IDataReader.GetRowParser<string>() throws exception #1822

p21x opened this issue Sep 6, 2022 · 2 comments · May be fixed by #1824

Comments

@p21x
Copy link

p21x commented Sep 6, 2022

Using .NET Core 6
Dapper v 2.0.123

using Dapper;
using Microsoft.Data.SqlClient;

using var connection = new SqlConnection("Server=localhost;Database=Test;Integrated Security=True;Encrypt=False");

var reader = connection.ExecuteReader("select 'test' as col");
var rowParser = reader.GetRowParser<string>();

reader.Read();
string result = rowParser(reader);

Throws exception:

System.InvalidCastException: 'Unable to cast object of type 'System.Func`2[System.Data.IDataReader,System.Object]' to type 'System.Func`2[System.Data.IDataReader,System.String]'.'

reader.GetRowParser<object>(typeof(string))

works, but then extra unboxing is needed: string result = (string)rowParser(reader)
@olivier-spinelli
Copy link
Contributor

You're right: this is a bug (but FYI, this is not an "extra unboxing", just a downcast). The first thing is to write a unit test for it (in DataReaderTests):

        /// <summary>
        /// See https://github.com/DapperLib/Dapper/issues/1822
        /// </summary>
        [Fact]
        public void issue_number_1822()
        {
            var reader = connection.ExecuteReader("select 'test' as col");
            var rowParser = reader.GetRowParser<string>();

            reader.Read();
            // This should not throw!
            string result = rowParser(reader);
            Assert.Equal("test", result);
        }

And it's awfully red.

My firt idea to fix it was simply to rewrite GetRowParser like this:

        public static Func<IDataReader, T> GetRowParser<T>(this IDataReader reader, Type concreteType = null,
            int startIndex = 0, int length = -1, bool returnNullIfFirstMissing = false)
        {
            concreteType ??= typeof(T);
            var func = GetDeserializer(concreteType, reader, startIndex, length, returnNullIfFirstMissing);
            // Always use a wrapper: this works for value and ref types (delegate casting is not an option).
            return _ => (T)func(_);
        }

Instead of:

        public static Func<IDataReader, T> GetRowParser<T>(this IDataReader reader, Type concreteType = null,
            int startIndex = 0, int length = -1, bool returnNullIfFirstMissing = false)
        {
            concreteType ??= typeof(T);
            var func = GetDeserializer(concreteType, reader, startIndex, length, returnNullIfFirstMissing);
            if (concreteType.IsValueType)
            {
                return _ => (T)func(_);
            }
            else
            {
                return (Func<IDataReader, T>)(Delegate)func;
            }
        }

But unfortunately, this breaks the GetSameReaderForSameShape test. To make it happy (and avoid a wrapper that is a good thing), the actual fix is slighty more involved:

        public static Func<IDataReader, T> GetRowParser<T>(this IDataReader reader, Type concreteType = null,
            int startIndex = 0, int length = -1, bool returnNullIfFirstMissing = false)
        {
            concreteType ??= typeof(T);
            var func = GetDeserializer(concreteType, reader, startIndex, length, returnNullIfFirstMissing);
            if (func.Method.ReturnType != typeof(T))
            { // Always use a wrapper: this works for value and ref types (delegate casting is not an option).
                return _ => (T)func(_);
            }
            return (Func<IDataReader, T>)(Delegate)func;
        }

I'll submit a PR asap.

olivier-spinelli added a commit to signature-opensource/CK-Dapper that referenced this issue Sep 22, 2022
@olivier-spinelli
Fix DapperLib#1822
c1f8541
@olivier-spinelli olivier-spinelli linked a pull request Sep 22, 2022 that will close this issue
Open
@arj03
Copy link

arj03 commented Sep 7, 2023

Just ran into this bug. Thanks for fixing @olivier-spinelli

@olivier-spinelli olivier-spinelli mentioned this issue Oct 3, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix IDataReader.GetRowParser<string>() throws exception #1822 signature-opensource/CK-Dapper
3 participants
@arj03 @olivier-spinelli @p21x