Skip to content

Commit

Permalink
Don't specify dbtype for DateTime et al; fixed npgsql 6 issue (#1723)
Browse files Browse the repository at this point in the history
* context: #1716 #1716

- change type-map to allow nullable
- change DateTime/TimeSpan to null
- do not specify DbType for null
- semi-breaking change: swap GetDbType to SetDbType, noting that it is marked [Obsolete] and internal-use-only
- semi-breaking change: make LookupDbType return nullable (same internal-use/[Obsolete])

* add test for Npgsql 6.0.0-rc.1

* try list parameters (like arrays)

* never assign -1 (sentinel) as the .DbType

* postgres tests
  • Loading branch information
mgravell authored Nov 3, 2021
1 parent 80719d0 commit b272cc6
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 36 deletions.
20 changes: 10 additions & 10 deletions Dapper/DynamicParameters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,12 @@ void SqlMapper.IDynamicParameters.AddParameters(IDbCommand command, SqlMapper.Id
/// </summary>
public bool RemoveUnused { get; set; }

internal static bool ShouldSetDbType(DbType? dbType)
=> dbType.HasValue && dbType.GetValueOrDefault() != EnumerableMultiParameter;

internal static bool ShouldSetDbType(DbType dbType)
=> dbType != EnumerableMultiParameter; // just in case called with non-nullable

/// <summary>
/// Add all the parameters needed to the command just before it executes
/// </summary>
Expand Down Expand Up @@ -262,9 +268,9 @@ protected void AddParameters(IDbCommand command, SqlMapper.Identity identity)
#pragma warning disable 0618
p.Value = SqlMapper.SanitizeParameterValue(val);
#pragma warning restore 0618
if (dbType != null && p.DbType != dbType)
if (ShouldSetDbType(dbType) && p.DbType != dbType.GetValueOrDefault())
{
p.DbType = dbType.Value;
p.DbType = dbType.GetValueOrDefault();
}
var s = val as string;
if (s?.Length <= DbString.DefaultLength)
Expand All @@ -277,7 +283,7 @@ protected void AddParameters(IDbCommand command, SqlMapper.Identity identity)
}
else
{
if (dbType != null) p.DbType = dbType.Value;
if (ShouldSetDbType(dbType)) p.DbType = dbType.GetValueOrDefault();
if (param.Size != null) p.Size = param.Size.Value;
if (param.Precision != null) p.Precision = param.Precision.Value;
if (param.Scale != null) p.Scale = param.Scale.Value;
Expand Down Expand Up @@ -468,15 +474,9 @@ static void ThrowInvalidChain()
}
else
{
dbType = (!dbType.HasValue)
#pragma warning disable 618
? SqlMapper.LookupDbType(targetMemberType, targetMemberType?.Name, true, out SqlMapper.ITypeHandler handler)
#pragma warning restore 618
: dbType;

// CameFromTemplate property would not apply here because this new param
// Still needs to be added to the command
Add(dynamicParamName, expression.Compile().Invoke(target), null, ParameterDirection.InputOutput, sizeToSet);
Add(dynamicParamName, expression.Compile().Invoke(target), dbType, ParameterDirection.InputOutput, sizeToSet);
}

parameter = parameters[dynamicParamName];
Expand Down
56 changes: 32 additions & 24 deletions Dapper/SqlMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,11 @@ where pair.Value > 1
select Tuple.Create(pair.Key, pair.Value);
}

private static Dictionary<Type, DbType> typeMap;
private static Dictionary<Type, DbType?> typeMap;

static SqlMapper()
{
typeMap = new Dictionary<Type, DbType>(37)
typeMap = new Dictionary<Type, DbType?>(37)
{
[typeof(byte)] = DbType.Byte,
[typeof(sbyte)] = DbType.SByte,
Expand All @@ -184,9 +184,9 @@ static SqlMapper()
[typeof(string)] = DbType.String,
[typeof(char)] = DbType.StringFixedLength,
[typeof(Guid)] = DbType.Guid,
[typeof(DateTime)] = DbType.DateTime,
[typeof(DateTime)] = null,
[typeof(DateTimeOffset)] = DbType.DateTimeOffset,
[typeof(TimeSpan)] = DbType.Time,
[typeof(TimeSpan)] = null,
[typeof(byte[])] = DbType.Binary,
[typeof(byte?)] = DbType.Byte,
[typeof(sbyte?)] = DbType.SByte,
Expand All @@ -202,9 +202,9 @@ static SqlMapper()
[typeof(bool?)] = DbType.Boolean,
[typeof(char?)] = DbType.StringFixedLength,
[typeof(Guid?)] = DbType.Guid,
[typeof(DateTime?)] = DbType.DateTime,
[typeof(DateTime?)] = null,
[typeof(DateTimeOffset?)] = DbType.DateTimeOffset,
[typeof(TimeSpan?)] = DbType.Time,
[typeof(TimeSpan?)] = null,
[typeof(object)] = DbType.Object
};
ResetTypeHandlers(false);
Expand Down Expand Up @@ -234,9 +234,9 @@ public static void AddTypeMap(Type type, DbType dbType)
// use clone, mutate, replace to avoid threading issues
var snapshot = typeMap;

if (snapshot.TryGetValue(type, out DbType oldValue) && oldValue == dbType) return; // nothing to do
if (snapshot.TryGetValue(type, out var oldValue) && oldValue == dbType) return; // nothing to do

typeMap = new Dictionary<Type, DbType>(snapshot) { [type] = dbType };
typeMap = new Dictionary<Type, DbType?>(snapshot) { [type] = dbType };
}

/// <summary>
Expand All @@ -250,7 +250,7 @@ public static void RemoveTypeMap(Type type)

if (!snapshot.ContainsKey(type)) return; // nothing to do

var newCopy = new Dictionary<Type, DbType>(snapshot);
var newCopy = new Dictionary<Type, DbType?>(snapshot);
newCopy.Remove(type);

typeMap = newCopy;
Expand Down Expand Up @@ -336,15 +336,20 @@ public static void AddTypeHandlerImpl(Type type, ITypeHandler handler, bool clon
/// <summary>
/// Get the DbType that maps to a given value.
/// </summary>
/// <param name="parameter">The parameter to configure the value for.</param>
/// <param name="value">The object to get a corresponding database type for.</param>
[Obsolete(ObsoleteInternalUsageOnly, false)]
[Browsable(false)]
[EditorBrowsable(EditorBrowsableState.Never)]
public static DbType GetDbType(object value)
public static void SetDbType(IDataParameter parameter, object value)
{
if (value == null || value is DBNull) return DbType.Object;
if (value == null || value is DBNull) return;

return LookupDbType(value.GetType(), "n/a", false, out ITypeHandler _);
var dbType = LookupDbType(value.GetType(), "n/a", false, out ITypeHandler _);
if (DynamicParameters.ShouldSetDbType(dbType))
{
parameter.DbType = dbType.GetValueOrDefault();
}
}

/// <summary>
Expand All @@ -357,7 +362,7 @@ public static DbType GetDbType(object value)
[Obsolete(ObsoleteInternalUsageOnly, false)]
[Browsable(false)]
[EditorBrowsable(EditorBrowsableState.Never)]
public static DbType LookupDbType(Type type, string name, bool demand, out ITypeHandler handler)
public static DbType? LookupDbType(Type type, string name, bool demand, out ITypeHandler handler)
{
handler = null;
var nullUnderlyingType = Nullable.GetUnderlyingType(type);
Expand All @@ -366,7 +371,7 @@ public static DbType LookupDbType(Type type, string name, bool demand, out IType
{
type = Enum.GetUnderlyingType(type);
}
if (typeMap.TryGetValue(type, out DbType dbType))
if (typeMap.TryGetValue(type, out var dbType))
{
return dbType;
}
Expand Down Expand Up @@ -2031,7 +2036,7 @@ public static void PackListParameters(IDbCommand command, string namePrefix, obj
var count = 0;
bool isString = value is IEnumerable<string>;
bool isDbString = value is IEnumerable<DbString>;
DbType dbType = 0;
DbType? dbType = null;

int splitAt = SqlMapper.Settings.InListStringSplitCount;
bool viaSplit = splitAt >= 0
Expand Down Expand Up @@ -2076,9 +2081,9 @@ public static void PackListParameters(IDbCommand command, string namePrefix, obj
if (tmp != null && !(tmp is DBNull))
lastValue = tmp; // only interested in non-trivial values for padding

if (listParam.DbType != dbType)
if (DynamicParameters.ShouldSetDbType(dbType) && listParam.DbType != dbType.GetValueOrDefault())
{
listParam.DbType = dbType;
listParam.DbType = dbType.GetValueOrDefault();
}
command.Parameters.Add(listParam);
}
Expand All @@ -2092,7 +2097,10 @@ public static void PackListParameters(IDbCommand command, string namePrefix, obj
var padParam = command.CreateParameter();
padParam.ParameterName = namePrefix + count.ToString();
if (isString) padParam.Size = DbString.DefaultLength;
padParam.DbType = dbType;
if (DynamicParameters.ShouldSetDbType(dbType))
{
padParam.DbType = dbType.GetValueOrDefault();
}
padParam.Value = lastValue;
command.Parameters.Add(padParam);
}
Expand Down Expand Up @@ -2528,7 +2536,7 @@ internal static Action<IDbCommand, object> CreateParamInfoGenerator(Identity ide
continue;
}
#pragma warning disable 618
DbType dbType = LookupDbType(prop.PropertyType, prop.Name, true, out ITypeHandler handler);
DbType? dbType = LookupDbType(prop.PropertyType, prop.Name, true, out ITypeHandler handler);
#pragma warning restore 618
if (dbType == DynamicParameters.EnumerableMultiParameter)
{
Expand Down Expand Up @@ -2563,22 +2571,22 @@ internal static Action<IDbCommand, object> CreateParamInfoGenerator(Identity ide
il.Emit(OpCodes.Ldstr, prop.Name); // stack is now [parameters] [parameters] [parameter] [parameter] [name]
il.EmitCall(OpCodes.Callvirt, typeof(IDataParameter).GetProperty(nameof(IDataParameter.ParameterName)).GetSetMethod(), null);// stack is now [parameters] [parameters] [parameter]
}
if (dbType != DbType.Time && handler == null) // https://connect.microsoft.com/VisualStudio/feedback/details/381934/sqlparameter-dbtype-dbtype-time-sets-the-parameter-to-sqldbtype-datetime-instead-of-sqldbtype-time
if (DynamicParameters.ShouldSetDbType(dbType) && dbType != DbType.Time && handler == null) // https://connect.microsoft.com/VisualStudio/feedback/details/381934/sqlparameter-dbtype-dbtype-time-sets-the-parameter-to-sqldbtype-datetime-instead-of-sqldbtype-time
{
il.Emit(OpCodes.Dup);// stack is now [parameters] [[parameters]] [parameter] [parameter]
if (dbType == DbType.Object && prop.PropertyType == typeof(object)) // includes dynamic
if (dbType.GetValueOrDefault() == DbType.Object && prop.PropertyType == typeof(object)) // includes dynamic
{
// look it up from the param value
il.Emit(OpCodes.Ldloc, typedParameterLocal); // stack is now [parameters] [[parameters]] [parameter] [parameter] [typed-param]
il.Emit(callOpCode, prop.GetGetMethod()); // stack is [parameters] [[parameters]] [parameter] [parameter] [object-value]
il.Emit(OpCodes.Call, typeof(SqlMapper).GetMethod(nameof(SqlMapper.GetDbType), BindingFlags.Static | BindingFlags.Public)); // stack is now [parameters] [[parameters]] [parameter] [parameter] [db-type]
il.Emit(OpCodes.Call, typeof(SqlMapper).GetMethod(nameof(SqlMapper.SetDbType), BindingFlags.Static | BindingFlags.Public)); // stack is now [parameters] [[parameters]] [parameter]
}
else
{
// constant value; nice and simple
EmitInt32(il, (int)dbType);// stack is now [parameters] [[parameters]] [parameter] [parameter] [db-type]
EmitInt32(il, (int)dbType.GetValueOrDefault());// stack is now [parameters] [[parameters]] [parameter] [parameter] [db-type]
il.EmitCall(OpCodes.Callvirt, typeof(IDataParameter).GetProperty(nameof(IDataParameter.DbType)).GetSetMethod(), null);// stack is now [parameters] [[parameters]] [parameter]
}
il.EmitCall(OpCodes.Callvirt, typeof(IDataParameter).GetProperty(nameof(IDataParameter.DbType)).GetSetMethod(), null);// stack is now [parameters] [[parameters]] [parameter]
}

il.Emit(OpCodes.Dup);// stack is now [parameters] [[parameters]] [parameter] [parameter]
Expand Down
1 change: 1 addition & 0 deletions nuget.config
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<configuration>
<packageSources>
<clear />
<add key="npgsql" value="https://www.myget.org/F/npgsql-unstable/api/v3/index.json" />
<add key="NuGet" value="https://api.nuget.org/v3/index.json" />
</packageSources>
</configuration>
2 changes: 1 addition & 1 deletion tests/Dapper.Tests/Dapper.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<PackageReference Include="Microsoft.Data.Sqlite" Version="5.0.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.8.0" />
<PackageReference Include="MySqlConnector" Version="1.1.0" />
<PackageReference Include="Npgsql" Version="5.0.0" />
<PackageReference Include="Npgsql" Version="6.0.0-rtm-ci.20211103T101701" />
<PackageReference Include="Snowflake.Data" Version="2.0.3" />
<PackageReference Include="System.Data.SqlClient" Version="4.8.2" />
<PackageReference Include="System.ValueTuple" Version="4.5.0" />
Expand Down
30 changes: 29 additions & 1 deletion tests/Dapper.Tests/Providers/PostgresqlTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Data;
using System.Data.Common;
using System.Linq;
Expand Down Expand Up @@ -44,7 +45,7 @@ public void TestPostgresqlArrayParameters()
{
IDbTransaction transaction = conn.BeginTransaction();
conn.Execute("create table tcat ( id serial not null, breed character varying(20) not null, name character varying (20) not null);");
conn.Execute("insert into tcat(breed, name) values(:breed, :name) ", Cats);
conn.Execute("insert into tcat(breed, name) values(:Breed, :Name) ", Cats);

var r = conn.Query<Cat>("select * from tcat where id=any(:catids)", new { catids = new[] { 1, 3, 5 } });
Assert.Equal(3, r.Count());
Expand All @@ -55,6 +56,24 @@ public void TestPostgresqlArrayParameters()
}
}

[FactPostgresql]
public void TestPostgresqlListParameters()
{
using (var conn = GetOpenNpgsqlConnection())
{
IDbTransaction transaction = conn.BeginTransaction();
conn.Execute("create table tcat ( id serial not null, breed character varying(20) not null, name character varying (20) not null);");
conn.Execute("insert into tcat(breed, name) values(:Breed, :Name) ", new List<Cat>(Cats));

var r = conn.Query<Cat>("select * from tcat where id=any(:catids)", new { catids = new List<int> { 1, 3, 5 } });
Assert.Equal(3, r.Count());
Assert.Equal(1, r.Count(c => c.Id == 1));
Assert.Equal(1, r.Count(c => c.Id == 3));
Assert.Equal(1, r.Count(c => c.Id == 5));
transaction.Rollback();
}
}

private class CharTable
{
public int Id { get; set; }
Expand Down Expand Up @@ -88,6 +107,15 @@ public void TestPostgresqlSelectArray()
}
}

[FactPostgresql]
public void TestPostgresqlDateTimeUsage()
{
using (var conn = GetOpenNpgsqlConnection())
{
_ = conn.ExecuteScalar("SELECT @Now", new { Now = DateTime.UtcNow });
}
}

[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
public class FactPostgresqlAttribute : FactAttribute
{
Expand Down

0 comments on commit b272cc6

Please sign in to comment.