Donald icon indicating copy to clipboard operation
Donald copied to clipboard

Require help on query

Open AnalyzeNCode opened this issue 3 years ago • 4 comments

Hi, Is there any example for IN query in Donald? For ex. I have customerId of type GUID, and I want to get customers by passing Ids as parameters and my query is like "SELECT ID, Name FROM ClientMaster WHERE ID in (@ids)”

I have tried by converting the list of GUIDs into list of string and concatenate all the strings into single string, but that's not working and only returning first result.

An example of parameter would be helpful. Thanks in advance.

AnalyzeNCode avatar Apr 11 '22 17:04 AnalyzeNCode

I don't have an example right now, but it should be almost the same as with typical command parameter: Take a look at this answer: Pass Array Parameter in SqlCommand

Let me know if won't be enough to make it work.

FoggyFinder avatar Apr 11 '22 18:04 FoggyFinder

Thanks, @FoggyFinder I follow the example and implemented the similar approach. Below is my implementation and it's working.

`let sqlParams = customerIds |> List.mapi (fun index custId -> let param = sprintf "@custId%i" index (param, SqlType.Int custId))

    let paramsQuery =
        sqlParams
        |> List.fold
            (fun acc (param, _) ->
                (match acc |> String.IsNullOrWhiteSpace with
                 | true -> acc
                 | false -> acc + ",")
                + param)
            String.Empty

    let sql =
        $"select CustomerID, FirstName, LastName, CompanyName from [SalesLT].[Customer] where CustomerID in ({paramsQuery})
        select CustomerID, AddressID, rowguid from [SalesLT].[CustomerAddress] where CustomerID in ({paramsQuery})"`

AnalyzeNCode avatar Apr 12 '22 06:04 AnalyzeNCode

I apologize @AnalyzeNCode, normally I like to respond to issues as quickly as possible. But I've been very ill for the past several days, so just catching up.

The answer shared by @FoggyFinder is exactly how you would accomplish this. I do wonder whether this type of convenience function has value to thers. Generally I like to avoid this pattern, since to me it's a code smell. But I do completely acknowledge times there are no suitable alternatives. What do you two think?

As for the implementation, your code will work. But it is vulnerable to SQL injections. You should paramerize the inputs. Here is a full working example script that parses an int input list into params and a SQL literaral.

#r "nuget: Donald"
#r "nuget: System.Data.SQLite"

open System
open System.Data
open Donald
open System.Data.SQLite

let connectionString = "Data Source=:memory:;Version=3;New=True;"
let connection = 
    let conn = new SQLiteConnection(connectionString)
    conn.Open()
    conn

let inputs = [1..10]

let param = 
    [
        for i in inputs do
            sprintf "@%i" i, SqlType.Int32 i
    ]

let paramSql = 
    [ 
        for p in param do 
            fst p
    ]
    |> String.concat ", "    

let sql = "SELECT " + paramSql

printfn "%s" sql
printfn "%A" param

connection
|> Db.newCommand sql
|> Db.setParams param
|> Db.query (fun rd -> 
    [ 
        for i = 0 to inputs.Length - 1 do 
            rd.GetInt32(i)
    ])
|> printfn "%A"

pimbrouwers avatar Apr 13 '22 10:04 pimbrouwers

Hey, @pimbrouwers, Thanks for the reply and suggestion. Sorry, about your health, I hope you are doing well now. Get well soon.

Actually, it's my mistake as I did post the partial code, below is my complete code with parameterized query.

    let getPersons customerIds =
             let sqlParams =
                customerIds
                |> List.mapi (fun index custId ->
                    let param = sprintf "@custId%i" index
                    (param, SqlType.Int custId))

    let paramsQuery =
        sqlParams
        |> List.fold
            (fun acc (param, _) ->
                (match acc |> String.IsNullOrWhiteSpace with
                 | true -> acc
                 | false -> acc + ",")
                + param)
            String.Empty

    let sql =
        $"select CustomerID, FirstName, LastName, CompanyName from [SalesLT].[Customer] where CustomerID in ({paramsQuery})
        select CustomerID, AddressID, rowguid from [SalesLT].[CustomerAddress] where CustomerID in ({paramsQuery})"

    use conn = new SqlConnection(ConnectionString)

    conn
    |> Db.newCommand sql
    |> Db.setParams sqlParams
    |> Db.query Customer.ofDataReader`

Regarding the point you mentioned, that such kind of implementation is like code smell.

I am unable to understand why it's code smell as my requirements are very simple and implementation is mostly simple CRUD based implementation and there is nothing like domain logic so no aggregates etc.

But if the SQL query with “IN” clause is a kind of code smell, then as per my requirement, I don't find any better alternatives for such type of queries.

In my current project, in many situations I have to query like this.

AnalyzeNCode avatar Apr 13 '22 12:04 AnalyzeNCode

The "smell" aspect comes from the need to pass in a complex parameter, like a list. The "IN" aspect of the query is not problematic per se, I just prefer to inject the values individually using the simplistic built-in mechanisms.

pimbrouwers avatar Aug 16 '22 16:08 pimbrouwers

If there are multiple comma separated values inside a single variable for the IN query, in that kind of case I generally prefer the plain SQL query with join. Approx benchmark to pass values as IN query using single parameter is around 10 values.

AnalyzeNCode avatar Aug 17 '22 16:08 AnalyzeNCode

That won't work the way you expect. You cannot pass a comma separated value list via a single SQL parameter. You can of course add it to the SQL string directly, but that is a horrible idea because it completely exposes you to a SQL injection. If you want to build the SQL string dynamically like that, you are best off using a SQL builder, like this one. Then you can loop through your IN values and use the WhereOr function.

pimbrouwers avatar Aug 17 '22 23:08 pimbrouwers