-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Description
First, much to Dapper's credit, amongst the other wonderful functionality it providers, it provides a way for all of its supported databases to be able to use arrays as parameters, regardless of whether the underlying driver provides such support itself or not. This allows for easy and safe passing of an arbitrary number of parameters without requiring consumers to fall back on string concatenation. This is definitely an awesome feature, but due to an under-the-hood detail in how this expansion is performed, writing SQL that uses arrays and is intended to work with multiple database providers is problematic.
The crux of the issue is that, when performing expansion, Dapper conditionally adds a set of parenthesis around the section being expanded. Taking the example from Readme.md:
Dapper allows you to pass in
IEnumerable<int>
and will automatically parameterize your query.For example:
connection.Query<int>("select * from (select 1 as Id union all select 2 union all select 3) as X where Id in @Ids", new { Ids = new int[] { 1, 2, 3 } });Will be translated to:
select * from (select 1 as Id union all select 2 union all select 3) as X where Id in (@Ids1, @Ids2, @Ids3)" // @Ids1 = 1 , @Ids2 = 2 , @Ids2 = 3
However, Dapper does not unconditionally attempt to perform expansion, but only does so if the underlying driver lacks support for arrays. In the event that the driver does support arrays, no expansion is performed, and the original SQL is passed to the database driver as-is.
Lines 1988 to 2002 in 6ec3804
public static void PackListParameters(IDbCommand command, string namePrefix, object value) | |
{ | |
// initially we tried TVP, however it performs quite poorly. | |
// keep in mind SQL support up to 2000 params easily in sp_executesql, needing more is rare | |
if (FeatureSupport.Get(command.Connection).Arrays) | |
{ | |
var arrayParm = command.CreateParameter(); | |
arrayParm.Value = SanitizeParameterValue(value); | |
arrayParm.ParameterName = namePrefix; | |
command.Parameters.Add(arrayParm); | |
} | |
else | |
{ | |
bool byPosition = ShouldPassByPosition(command.CommandText); |
... skipping further into the else ...
Lines 2121 to 2128 in 6ec3804
var sb = GetStringBuilder().Append('(').Append(variableName); | |
if (!byPosition) sb.Append(1); else sb.Append(namePrefix).Append(1).Append(variableName); | |
for (int i = 2; i <= count; i++) | |
{ | |
sb.Append(',').Append(variableName); | |
if (!byPosition) sb.Append(i); else sb.Append(namePrefix).Append(i).Append(variableName); | |
} | |
return sb.Append(')').ToStringRecycle(); |
The practical effect of this is that Dapper adds parenthesis around arrays for all databases except Postgres and ClickHouse, since those are the only two that are listed as supporting arrays (based on the values set in https://github.com/DapperLib/Dapper/blob/a31dfd3dd4d7f3f2580bd33c877199d7ef3e3ef9/Dapper/FeatureSupport.cs).
This means that, given the original SQL from the Readme.md, Postgres would attempt to execute where Id in @Ids
, whereas another database, such as Oracle, would attempt to run where Id in (@Ids1, @Ids2, @Ids3)
. Without the addition of parenthesis, the above is considered invalid. If, however, I add parenthesis to the original SQL so that it becomes valid for Postgres then the expanded form, as would run on Oracle, would read Id in ((@Ids1, @Ids2, @Ids3))
and would fail, due to the extra parenthesis.
In order to allow for a consistent interface for array handling that could be used by multiple database providers, would you be willing to consider the addition of a new SqlMapper.Setting
, possible ForceArrayExpansion
that could be enabled parameter expansion even if they driver already has array support? This would allow a path for consistent handling without breaking backwards-compatibility by changing default behavior.
If this would be acceptable, I'd be more than willing to implement it and provide a MR for critique.