fix null parameter being mistaken for the string type

[fredliex]

Always try to set the DbType of the pameter to avoid the null value being mistaken for the string type.

code

var data = connection.ExecuteQuery<decimal?>("select @value", new { value = (decimal?)null }).First();

exception

System.InvalidOperationException
  HResult=0x80131509
  Message=No coercion operator is defined between types 'System.String' and 'System.Nullable`1[System.Decimal]'.
  Source=System.Linq.Expressions
  StackTrace: 
   at System.Linq.Expressions.Expression.GetUserDefinedCoercionOrThrow(ExpressionType coercionType, Expression expression, Type convertToType)
   at System.Linq.Expressions.Expression.Convert(Expression expression, Type type, MethodInfo method)
   at System.Linq.Expressions.Expression.Convert(Expression expression, Type type)
   at RepoDb.Reflection.Compiler.ConvertExpressionToTypeExpression(Expression expression, Type toType) in D:\Code-git\RepoDB\RepoDb.Core\RepoDb\Reflection\Compiler\Compiler.cs:line 574

analysis

Expression generated by Compiler.GetPlainTypeToDbParametersCompiledFunction is

DbCommand.CreateParameter("name", value, new Nullable<DbType>());

When the third parameter is not specified and the value is null, sql server will treat the parameter as a string.
So DbDataReader.GetFieldType(n) will return DbType.String, the read Expression will use DbDataReader.GetString(n), and then the string will be converted to the return type decimal and an error will occur.

So this PR is mainly to set Parameter.DbType as much as possible to avoid misjudgment of the type when it is null.

If you have any questions, please let me know, and I will modify it as soon as possible.

thank

[mikependon]

Thanks for this PR, we will review and revert to you soon. Btw, we just pushed the newly build and might be the last build of this year. So please let us know if this needs to be on beta release so we can issue it for you ASAP.

[fredliex]

No time pressure, no need for beta.

thank

[mikependon]

We do not need this additional mapping anymore as the code at the line 261 should do this already. If there are issues to this, we should fix it there somehow, not introducing this new mapping.

var dbType = (returnType != null ? clientTypeToDbTypeResolver.Resolve(returnType) : null) ??
                    classProperty.GetDbType() ??
                    value?.GetType()?.GetDbType();

[mikependon]

Same here, the line 520 should do it already.

[mikependon]

Same here, the line 333 should do it already.

[fredliex]

The processing method has been changed.
Use ClientTypeToDbTypeResolver instead of TypeMapper.fallback.
And Remove unnecessary logic.

Please review again.
thanks

[mikependon]

The logic is correct now, so I will accept this. Though, you can also remove the additional checking and make it single line like below.

// DbType
var dbType = (returnType != null ? clientTypeToDbTypeResolver.Resolve(returnType) : null) ??
	classProperty.GetDbType() ??
	value?.GetType()?.GetDbType() ??
        classProperty != null ? clientTypeToDbTypeResolver.Resolve(classProperty.PropertyInfo.PropertyType) : null;

[mikependon]

As part of our coding standard, you might notice that we are always doing a "new line" per argument.

private static DbType? GetDbType(ClassProperty classProperty,
      Type dbFieldType)
{
     ...
}

But I am happy that you are very engage here, I know that next time, you will be one of my biggest contributor and/or active collaborator. Thanks as always 😺

[mikependon]

I just finished the review for this and I will do the merge now 😺 . Though it is up to you if you would like to make an additional push on the minor comments I made, but the logic is now okay and reviewed.

Thank you always and please continue to do and maturize this library 🚀