DapperLib / DapperLib/Dapper

Wrong type supplied to custom converter when working with SQLite database

Open
#1,607 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
18.4k
Forks
3.7k
Avg merge
5h 8m
Merged PRs (30d)
1

Description

I have been working with SQLite database using Dapper from F# and wanted to use a custom converter that looks like this:

type OptionHandler<'T> () =
    inherit SqlMapper.TypeHandler<option<'T>> ()

    override __.SetValue (param, value) =
        let valueOrNull =
            match value with
            | Some x -> box x
            | None   -> null

        param.Value <- valueOrNull

    override __.Parse value =
        if 
            Object.ReferenceEquals(value, null) || 
            value = box DBNull.Value
        then None
        else Some (value :?> 'T)

(credit to https://github.com/AlexTroshkin/fsharp-dapper)
And a simple record that looks like this:

    [<CLIMutable>]
    type Measurement = { Id: int option; Name: string }

After adding several of such type handlers (e.g. SqlMapper.AddTypeHandler (OptionHandler<int32>()) and trying to query data using connection.QueryAsync(...) I have gotten an exception

System.AggregateException: One or more errors occurred. (Error parsing column 0 (Id=1 - Int64))
       ---> System.Data.DataException: Error parsing column 0 (Id=1 - Int64)
       ---> System.InvalidCastException: Unable to cast object of type 'System.Int64' to type 'System.Int32'.
         at Microsoft.FSharp.Core.LanguagePrimitives.IntrinsicFunctions.UnboxGeneric[T](Object source) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\prim-types.fs:line 613
         at FSharp.Data.Dapper.OptionHandler`1.Parse(Object value) in D:\Documents\Projects\F#\barkirv2\src\FsharpDapper\OptionHandler.fs:line 22
         at Dapper.SqlMapper.TypeHandler`1.Dapper.SqlMapper.ITypeHandler.Parse(Type destinationType, Object value) in D:\Documents\Projects\F#\Dapper\Dapper\SqlMapper.TypeHandler.cs:line 42
         at Dapper.SqlMapper.TypeHandlerCache`1.Parse(Object value) in D:\Documents\Projects\F#\Dapper\Dapper\SqlMapper.TypeHandlerCache.cs:line 23
         at Deserializec7035d31-bb55-4e15-8777-42214bc2c31d(IDataReader )
         --- End of inner exception stack trace ---
         at Dapper.SqlMapper.ThrowDataException(Exception ex, Int32 index, IDataReader reader, Object value) in D:\Documents\Projects\F#\Dapper\Dapper\SqlMapper.cs:line 3665
         at Deserializec7035d31-bb55-4e15-8777-42214bc2c31d(IDataReader )
         at Dapper.SqlMapper.QueryAsync[T](IDbConnection cnn, Type effectiveType, CommandDefinition command)
         --- End of inner exception stack trace --- 

(Callstack already contains Dapper source code as I'm writing this afterwards)

OptionHandler.fs line 22 is this line:
else Some (value :?> 'T)
Here mapper tries to make an explicit cast to 'T (in my situation, 'T was Int32) but the value was of type Int64
I have tried to investigate that and tried to debug Dapper source code and I have found that indeed the type returned from raw.sqlite3_column_type is indeed corresponds to Int64 but somewhere in Dapper code a conversion was made from Int64 to Int32. After further investigation I have found method SqlMapper.LoadReaderValueOrBranchToDBNullLabel. The interesting lines are here (3486 - 3512):

3486	 else
3487	                {
3488	                    TypeCode dataTypeCode = Type.GetTypeCode(colType), unboxTypeCode = Type.GetTypeCode(unboxType);
3489	                    bool hasTypeHandler;
3490	                    if ((hasTypeHandler = typeHandlers.ContainsKey(unboxType)) || colType == unboxType || dataTypeCode == unboxTypeCode || dataTypeCode == Type.GetTypeCode(nullUnderlyingType))
3491	                    {
3492	                        if (hasTypeHandler)
3493	                        {
3494	#pragma warning disable 618
3495	                            il.EmitCall(OpCodes.Call, typeof(TypeHandlerCache<>).MakeGenericType(unboxType).GetMethod(nameof(TypeHandlerCache<int>.Parse)), null); // stack is now [...][typed-value]
3496	#pragma warning restore 618
3497	                        }
3498	                        else
3499	                        {
3500	                            il.Emit(OpCodes.Unbox_Any, unboxType); // stack is now [...][typed-value]
3501	                        }
3502	                    }
3503	                    else
3504	                    {
3505	                        // not a direct match; need to tweak the unbox
3506	                        FlexibleConvertBoxedFromHeadOfStack(il, colType, nullUnderlyingType ?? unboxType, null);
3507	                        if (nullUnderlyingType != null)
3508	                        {
3509	                            il.Emit(OpCodes.Newobj, unboxType.GetConstructor(new[] { nullUnderlyingType })); // stack is now [...][typed-value]
3510	                        }
3511	                    }
3512	                }

If we had a regular Int32, the hasTypeHandler would have been false and we would have used FlexibleConvertBoxedFromHeadOfStack on line 3506 but because we have a converter hasTypeHandler is true and it tries to call the converter directly (without converting value to Int32 first) on line 3495

I would like this issue to be confirmed and maybe fixed. I am ready to provide relevant source code:
I have added a test to SqliteTests.cs:

// Usings omitted
using FSharp.Data.Dapper;
// Code omitted
  [FactSqlite]
        public void TestInt64WithMappers()
        {
            using (var connection = GetSQLiteConnection())
            {
                connection.Execute(@"CREATE TABLE 'Measurement' (
                    'Id'    INTEGER,
                    'Name'  text NOT NULL,
                    PRIMARY KEY('Id' AUTOINCREMENT)
                )");
                OptionHandler.RegisterTypes();
                connection.Execute("INSERT INTO Measurement (Name) VALUES ('ml.')");
                //var id1 = connection.QuerySingleAsync<int>("SELECT Id FROM Measurement LIMIT 1").GetAwaiter().GetResult();
                var measuC = connection.QuerySingleAsync<MeasurementC>("SELECT * FROM Measurement LIMIT 1").GetAwaiter().GetResult();
                var measu = connection.QuerySingleAsync<Measurement>("SELECT * FROM Measurement LIMIT 1").GetAwaiter().GetResult();
                var stuff = connection.QueryAsync<Measurement>("SELECT * FROM Measurement").GetAwaiter().GetResult();
            }
        }

And created a simple F# project with Program.fs containing this code:

namespace FSharp.Data.Dapper

open System
open Dapper 

[<CLIMutable>]
type Measurement = {
    id: int option
    name: string
}

type MeasurementC() =
        member val Id: int option = None with get, set
        member val Name: string = "" with get, set

type OptionHandler<'T> () =
    inherit SqlMapper.TypeHandler<option<'T>> ()

    override __.SetValue (param, value) =
        let valueOrNull =
            match value with
            | Some x -> box x
            | None   -> null

        param.Value <- valueOrNull

    override __.Parse value =
        if 
            Object.ReferenceEquals(value, null) || 
            value = box DBNull.Value
        then None
        else Some (value :?> 'T)

module OptionHandler =
    
    let RegisterTypes () =
        SqlMapper.AddTypeHandler (OptionHandler<int32>())
        SqlMapper.AddTypeHandler (OptionHandler<uint32>())
        SqlMapper.AddTypeHandler (OptionHandler<int64>())
        SqlMapper.AddTypeHandler (OptionHandler<uint64>())

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in SqlMapper.cs at LoadReaderValueOrBranchToDBNullLabel, especially the type-handler branch around lines 3486-3506, then review the TestInt64WithMappers reproduction in SqliteTests.cs. Confirm the SQLite Int64 value and registered OptionHandler interaction, and use the test to verify the corrected mapping behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, sqlite
Domain
database
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.