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.

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

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

Open
rhuijben opened this issue Oct 14, 2024 · 2 comments · May be fixed by #1184
Open

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

rhuijben opened this issue Oct 14, 2024 · 2 comments · May be fixed by #1184
Assignees
Labels
bug Something isn't working

Comments

@rhuijben
Copy link

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 rhuijben added the bug Something isn't working label Oct 14, 2024
@rhuijben
Copy link
Author

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

@rhuijben
Copy link
Author

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);

@rhuijben rhuijben linked a pull request Oct 20, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants