SqlHydra icon indicating copy to clipboard operation
SqlHydra copied to clipboard

Question about multiple inserts, possibility to not enforce the "match"?

Open MangelMaxime opened this issue 1 year ago • 5 comments

On the readme there is this note:

To insert multiple entities in one query, use the entities operation in conjunction with the AtLeastOne type to ensure that at least one item exists in the collection. (The AtLeastOne forces you to handle the case where an empty collection is passed to entities which would throw a runtime exception.)

I am perhaps naive but would it not be possible for SqlHydra to check if the collection is empty or not?

If the collection is empty it could resolve directly without hitting the database.

If we consider this workflow HTTP Request -> Validate -> Do stuff with data (insert into DB) -> HTTP Response

I think that if the list of elements to insert should not be empty then the user should handle it from it's code in the validation phase.

This would avoid forcing the user to have a match instruction for something he already handled.

let private myQuery (request : Request.MyQuery) : Async<Response.MyQuery> =                

    task {
        match Request.MyQuery.validate request with
        | Ok requestValues ->
            let proceduresToInsert =
                requestValues.Procedures
                |> List.map (fun procedure -> {
                    dbo.Procedures.Id = -1
                    dbo.Procedures.Procedure = procedure
                }
                )
                |> AtLeastOne.tryCreate

            match proceduresToInsert with
            | Some proceduresToInsert ->
                do!
                    insertTask (Create DbContext.create) {
                        for c in Tables.procedures do
                            entities proceduresToInsert
                            excludeColumn c.Id
                    }
                    :> Task

            | None -> () // Redondant because already handled in the validation phase

        | Error errors ->
            return Response.MyQuery.InvalidRequest errors
    }
    |> Async.AwaitTask

would become

let private myQuery (request : Request.MyQuery) : Async<Response.MyQuery> =                

    task {
        match Request.MyQuery.validate request with
        | Ok requestValues ->
            let proceduresToInsert =
                requestValues.Procedures
                |> List.map (fun procedure -> {
                    dbo.Procedures.Id = -1
                    dbo.Procedures.Procedure = procedure
                }
                )

            do!
                insertTask (Create DbContext.create) {
                    for c in Tables.procedures do
                        entities proceduresToInsert
                        excludeColumn c.Id
                }
                :> Task

        | Error errors ->
            return Response.MyQuery.InvalidRequest errors
    }
    |> Async.AwaitTask

MangelMaxime avatar Sep 20 '22 14:09 MangelMaxime

While It would be possible to have the library do nothing, I like that it is more specific about disallowing an invalid input.

But I agree that in your case it does make the code unnecessarily cumbersome. I think the best solution would be for you to use a helper function:

/// Prepares a pre-validated list for multiple insert.
let multiple (lst: seq<'a>) = 
    lst |> AtLeastOne.tryCreate |> Option.get

Usage:

let private myQuery (request : Request.MyQuery) : Async<Response.MyQuery> =                

    task {
        match Request.MyQuery.validate request with
        | Ok requestValues ->
            let proceduresToInsert =
                requestValues.Procedures
                |> List.map (fun procedure -> {
                    dbo.Procedures.Id = -1
                    dbo.Procedures.Procedure = procedure
                })
                |> multiple

            do!
                insertTask (Create DbContext.create) {
                    for c in Tables.procedures do
                        entities proceduresToInsert
                        excludeColumn c.Id
                }
                :> Task

        | Error errors ->
            return Response.MyQuery.InvalidRequest errors
    }
    |> Async.AwaitTask

JordanMarr avatar Sep 20 '22 15:09 JordanMarr

You could also skip the helper function altogether and just call the option .Value property when setting entities:

let private myQuery (request : Request.MyQuery) : Async<Response.MyQuery> =                

    task {
        match Request.MyQuery.validate request with
        | Ok requestValues ->
            let proceduresToInsert =
                requestValues.Procedures
                |> List.map (fun procedure -> {
                    dbo.Procedures.Id = -1
                    dbo.Procedures.Procedure = procedure
                })
                |> AtLeastOne.tryCreate

            do!
                insertTask (Create DbContext.create) {
                    for c in Tables.procedures do
                        entities proceduresToInsert.Value
                        excludeColumn c.Id
                }
                :> Task

        | Error errors ->
            return Response.MyQuery.InvalidRequest errors
    }
    |> Async.AwaitTask

You may also be interested in this library if you are doing a lot of bulk inserts/updates/deletes: https://github.com/JordanMarr/SqlBulkTools.FSharp

JordanMarr avatar Sep 20 '22 18:09 JordanMarr

While It would be possible to have the library do nothing, I like that it is more specific about disallowing an invalid input.

I understand you view.

I didn't think about using Option.get or .Value this would indeed reduce the pain.

By curiosity, I tried to look at the internal of SqlHydra to see if I could add my own CEs operator to support seq<'T> directly and do nothing if empty. However, I don't think it is possible to extends it to do nothing right now right?

MangelMaxime avatar Sep 20 '22 19:09 MangelMaxime

It would be difficult to have it do nothing due to the fact that the InsertBuilder just creates a InsertQuery object that is not executed until it is passed to the QueryContext. So it would likely involve adding a validation error property to the InsertQuery and then updating the QueryContext Insert and InsertAsyncWithOptions methods to check that and bypass if invalid.

JordanMarr avatar Sep 20 '22 20:09 JordanMarr

Since you are pre-validating, another approach would be to extend the InsertBuilder to take a simple 'T seq:


type InsertBuilder<'Inserted, 'InsertReturn when 'InsertReturn : struct> with
    
    /// Sets multiple values for INSERT
    [<CustomOperation("multiple", MaintainsVariableSpace = true)>]
    member this.Multiple (qs: QuerySource<'T, InsertQuerySpec<'T, 'InsertReturn>>, entities: 'T seq) = 
        QuerySource<'T, InsertQuerySpec<'T, 'InsertReturn>>(
            { qs.Query with Entities = entities |> Seq.toList }
            , qs.TableMappings)

JordanMarr avatar Sep 20 '22 20:09 JordanMarr