mikependon / RepoDB

A hybrid ORM library for .NET.
Apache License 2.0
1.7k stars 125 forks source link

Bug: SqlServer: Using DateOnly nullable columns with ConversionType.Automatic #1183

Closed rhuijben closed 1 month ago

rhuijben commented 1 month ago

Bug Description

For some time we used DateOnly without any issue after we installed a conversion mapper

        PropertyHandlerMapper.Add<DateOnly, RepoDbDateOnlyPropertyHandler>();
        PropertyHandlerMapper.Add<DateOnly?, RepoDbNullableDateOnlyPropertyHandler>();

public class RepoDbDateOnlyPropertyHandler : IPropertyHandler<DateTime, DateOnly>
{
    public DateOnly Get(DateTime input, PropertyHandlerGetOptions options)
        => DateOnly.FromDateTime(input);

    public DateTime Set(DateOnly input, PropertyHandlerSetOptions options)
        => input.ToDateTime(TimeOnly.MinValue);
}

public class RepoDbNullableDateOnlyPropertyHandler : IPropertyHandler<DateTime?, DateOnly?>
{
    public DateOnly? Get(DateTime? dt, PropertyHandlerGetOptions options)
        => dt.HasValue ? DateOnly.FromDateTime(dt.Value) : null;

    public DateTime? Set(DateOnly? input, PropertyHandlerSetOptions options)
        => input.HasValue ? input.Value.ToDateTime(TimeOnly.MinValue) : null;
}

We use DateOnly and DateOnly nullable columns.

Now I enabled ConversionType.Automatic to allow using enum values that aren't mapped and suddenly quering nullable enum fields broke down hard.

I found that this flag enabled the builtin support for DateOnly, and was quite surprised that this breaks so hard

Exception Message:

System.ArgumentException: 'Expression of type 'System.DateOnly' cannot be used for parameter of type 'System.Object' of method 'System.DateTime ToDateTime(System.Object)' (Parameter 'arg0')'

This exception was originally thrown at this call stack:
    System.Dynamic.Utils.ExpressionUtils.ValidateOneArgument(System.Reflection.MethodBase, System.Linq.Expressions.ExpressionType, System.Linq.Expressions.Expression, System.Reflection.ParameterInfo, string, string, int)
    System.Linq.Expressions.Expression.Call(System.Reflection.MethodInfo, System.Linq.Expressions.Expression)
    RepoDb.Reflection.Compiler.ConvertExpressionToSystemConvertExpression(System.Linq.Expressions.Expression, System.Type)
    RepoDb.Reflection.Compiler.ConvertExpressionWithAutomaticConversion(System.Linq.Expressions.Expression, System.Type)
    RepoDb.Reflection.Compiler.GetPlainTypeToDbParametersCompiledFunction(System.Type, System.Type, RepoDb.DbFieldCollection)
    RepoDb.Reflection.FunctionFactory.GetPlainTypeToDbParametersCompiledFunction(System.Type, System.Type, RepoDb.DbFieldCollection)
    RepoDb.FunctionCache.PlainTypeToDbParametersCompiledFunctionCache.Get(System.Type, System.Type, RepoDb.DbFieldCollection)
    RepoDb.FunctionCache.GetPlainTypeToDbParametersCompiledFunction(System.Type, System.Type, RepoDb.DbFieldCollection)
    RepoDb.DbConnectionExtension.CreateDbCommandForExecutionInternal(System.Data.IDbConnection, string, object, System.Data.CommandType?, int?, System.Data.IDbTransaction, System.Type, RepoDb.DbFieldCollection, bool)
    RepoDb.DbConnectionExtension.CreateDbCommandForExecutionAsync(System.Data.IDbConnection, string, object, System.Data.CommandType?, int?, System.Data.IDbTransaction, System.Threading.CancellationToken, System.Type, RepoDb.DbFieldCollection, bool)
    ...
    [Call Stack Truncated]

Schema and Model:

Nothing special. Just a table with a nullable dateofbirth

CREATE TABLE Person( [ID] [int] IDENTITY(1,1) NOT NULL, [CreatedOn] datetimeoffset NOT NULL, [DateOfBirth] [date] NULL, PRIMARY KEY CLUSTERED ( [ID] ASC )


record class Person
{
  public int ID {get;set;}
  public DateTimeOffset CreatedOn { get; set; }
  public DateOnly? DateOfBirth { get; set; }
}

And also the model that corresponds the schema.

Your class model here.

The original PropertyHandlerMapper mapping at the start doesn't affect any of this anymore. The cleanest fix would be that RepoDB would just support DateOnly directly without work on our side with enum conversions enabled. But going back to a working PropertyHandlerMapper would work as well. But not being able to use undefined enum values xor not using DateOnly is really a problem.

Library Version:

Example: RepoDb v1.13.1 and RepoDb.SqlServer v1.13.1

rhuijben commented 1 month ago

The 'System.DateTime ToDateTime(System.Object)' (Parameter 'arg0')' looks strange as the conversion should use DateTime DateOnly.ToDateTime(DateOnly value) here.

rhuijben commented 1 month ago

The query that fails is just a simple


string query = @$"
        SELECT  *
        FROM    Person
        WHERE   DateOfBirth = @DateOfBirth";

return await sql.ExecuteQueryAsync<Person>(query, new
{
    DateOfBirth = (DateOnly?)new DateOnly(2024,1,1)
}, cancellationToken: cancellationToken);