Skip to content

Parameter #742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Parameter #742

wants to merge 4 commits into from

Conversation

michelbieleveld
Copy link

@michelbieleveld michelbieleveld commented Apr 9, 2025

Hi first thank you for working on this project ! Highly usable and nice API.

As mentioned in #666, ideally one would want to support true sql named binding. This change uses the already existing Variables, Bindings and NamedParameters, and adds DefineParameter used as shown in the unit tests

var query = new Query("Products")
                .DefineParameter("@a", 1)
                .DefineParameter("@b","b")
                .Where(q=> q.Where("a", "=", Variable("@a")))
                .OrWhere(q =>
                    q.Where("a", "=", Variable("@a"))
                        .Where("b",">",Variable("@b"))
                        .Where("a",">",0));

which now results in the generated SQL of

SELECT * FROM [Products] WHERE ([a] = @a) OR ([a] = @a AND [b] > @b AND [a] > @p3)

and importantly the named bindings only now occur once in the named bindings:

{
 { "@a", 1 },
 { "@b", "b" },
 { "@p3", 0 }
}

which does not happen with the variables. There should be no impact for the existing code as long as one does not use DefineParameter , all unit tests are passing including the mysql live tests. Added a docker-compose.yaml so people can easily test with the live database as done in the code. Not sure how was done before.

Note that this will not work with the standard WhereEnds etc functions as the sql syntax would not be '%@var' but '%' + @var . Considering that this feature is as you mention hidden and I have no need, at the moment, have not adjusted all those functions.

Let me know if you want to see any changes, would be happy to see this merged as I want to be able to do something like this for cursor based pagination (just for reference).

// 3. Lexicographic pagination WHERE using named Variables
            let queryWithPagination =
                if sortFields.Length = values.Length then
                                // 1. Define named variables once to reuse across conditions
                    let t =
                        (queryWithOrdering, List.indexed sortFields)
                        ||> List.fold (fun acc (i, (field, _)) ->
                            acc.DefineParameter($"@{field}", values.[i])
                        )
                    t.Where(fun q ->

                        [ 0 .. sortFields.Length - 1 ]
                        |> List.fold (fun (innerQ:Query) index ->
                            let prefixEquals =
                                [ 0 .. index - 1 ]
                                |> List.map (fun i ->
                                    let (f, _) = sortFields.[i]
                                    (f, "=", Variable($"@{f}"))
                                )

                            let (field, dir) = sortFields.[index]
                            let op = match dir with
                                     | SortDirection.Asc -> ">"
                                     | SortDirection.Desc -> "<"

                            let conditionGroup = prefixEquals @ [ (field, op, Variable($"@{field}")) ]

                            // All conditions inside an OrWhere group
                            innerQ.OrWhere(fun branch ->
                                conditionGroup
                                |> List.fold (fun acc (f, o, v) ->
                                    // acc.Where(f, o, v)
                                    acc.Where(f,o,Variable($"@{f}"))
                                ) branch
                            )

                        ) q

                    )
                else
                    queryWithOrdering

which works in my local version, obviously with more code and functions required.

@michelbieleveld
Copy link
Author

michelbieleveld commented Apr 10, 2025

Ok, the disadvantages I mentioned earlier in previous edits are no longer in play and code is much cleaner. Realized I could just use the helper method but needed the bindings first as an array. Also now compatible with any solution that uses differerent marker , @, for parameters. Removed renaming internally the variables, the user is now responsible for naming correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant