Dzoukr / Dapper.FSharp

Lightweight F# extension for StackOverflow Dapper with support for MSSQL, MySQL, PostgreSQL, and SQLite
MIT License
365 stars 35 forks source link

Fix case sensitivity in creation of where parameter names #13

Closed ssiltanen closed 3 years ago

ssiltanen commented 3 years ago

Evaluating where has an issue with case sensitivity. When you have two conditions for the same column but the column names are with different casing, parameter names share the same index and trust only case sensitivity. This apparently causes problems at least in MSSQL when actually executing queries.

How to repro:

// Works as expected and different parameters are separated with different index
let sql1, values1 =
    select {
        table "example"
        where (eq "column" 1 + eq "column" 2)
    } |> Dapper.FSharp.MSSQL.Deconstructor.select<{| id : string |}>
// val values1 : Map<string,obj> =
//   map [("Where_column1", 1); ("Where_column2", 2)]
// val sql1 : string =
//   "SELECT [id] FROM [example] WHERE ([column] = @Where_column1 A"+[29 chars]

// Does not work as expected as only case sensitivity separates parameters
let sql2, values2 =
    select {
        table "example"
        where (eq "column" 1 + eq "Column" 2)
    } |> Dapper.FSharp.MSSQL.Deconstructor.select<{| id : string |}>
// val values2 : Map<string,obj> =
//   map [("Where_Column1", 2); ("Where_column1", 1)]
// val sql2 : string =
//   "SELECT [id] FROM [example] WHERE ([column] = @Where_column1 A"+[29 chars]

How to fix: When creating parameters, the case of the column name should be ignored.

Without this PR, how users can mitigate: Make sure your column names have similar casing.

This example I provided above, might or might not cause an issue when querying against live database, since this is a dumbed down version of the code I had in production, but it shows the output difference between different case names.

On another note, I noticed a minor performance improvement in the same function as this fix. I did not change it in this PR as it has nothing to do with it, and because I was not sure if there is a reason parameters are appended at the end of meta list. But if the order in the list does not matter, it is more efficient to add to the head of the list compared to creating a new list of one value and appending it at the end of existing list.

// Change this
meta @ [{ Key = (field, comp); Name = field; ParameterName = parName }]

// into this
{ Key = (field, comp); Name = field; ParameterName = parName } :: meta
Dzoukr commented 3 years ago

Thanks @ssiltanen! I'll also look at the performance thing you mentioned.

Dzoukr commented 3 years ago

Aaaaaaaand published in v1.12.2, thanks again!