Skip to content
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

Add a wrapper around ExecuteScalar #22

Closed
FoggyFinder opened this issue Jun 29, 2021 · 6 comments
Closed

Add a wrapper around ExecuteScalar #22

FoggyFinder opened this issue Jun 29, 2021 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@FoggyFinder
Copy link

It seems like currently one has to use querySingle etc that isn't convenient

@pimbrouwers
Copy link
Owner

pimbrouwers commented Jun 29, 2021

Hi @FoggyFinder,

Thanks for reaching out. Appreciate the interest.

I had the same line of thinking in the past. In fact, original iterations of this project had a invocation pathway for ExecuteScalar() which boxed return value based on the generic type parameter provided. This is obviously not an ideal situation for a number of reasons. So I modified the invocation to receive a mapping function to be applied against the obj returned. Again, less than ideal because an entire different suite of helpers had to be created to support that (in addition to those responsible for consuming IDataReader values) and it ultimately was clunky to deal with nulls.

So I decided that the optimal route was to leverage Db.QuerySingle which elegantly deals with all of the above. In addition to solving the problems above it elegantly deals with miscasts (yielding sensible exceptions) and has the benefit of streaming result data.

I had thought about creating a Db.Scalar function which simply calls Db.QuerySingle under the covers. But I haven't really found using QuerySingle to be clunky, and I use the library a lot for both work and personal projects.

Anyway, I'm open to hearing if there's a use case that I'm missing. But ultimately, I don't imagine I'll be adding an explicit wrapper around ExecuteScalar(). Make sense?

@pimbrouwers pimbrouwers self-assigned this Jun 29, 2021
@pimbrouwers pimbrouwers added the enhancement New feature or request label Jun 29, 2021
@FoggyFinder
Copy link
Author

Anyway, I'm open to hearing if there's a use case that I'm missing. But ultimately, I don't imagine I'll be adding an explicit wrapper around ExecuteScalar()

Well, right, I really don't care how it would work internally.

I had thought about creating a Db.Scalar function which simply calls Db.QuerySingle under the covers. But I haven't really found using QuerySingle to be clunky

It's just a bit annoying to check result first and then check whether the query is actually returned something.

I wrote a simple function though

module Db =
    let execSingle cmd = 
        match Db.querySingle (fun r -> r.GetValue(0) |> unbox) cmd with
        | DbResult.Ok r when r.IsSome -> 
            Result.Ok r.Value
        | DbResult.Ok _ ->
            Result.Error "A query didn't return any value"
        | DbResult.Error err ->
            Result.Error err.Statement

which I haven't tested yet but I believe it would work.

pimbrouwers added a commit that referenced this issue Jul 4, 2021
@pimbrouwers
Copy link
Owner

pimbrouwers commented Jul 4, 2021

Here's what I've come up with, which shares a signature with the functions I removed (i.e., (obj -> 'a) -> IDbCommand -> DbResult<'a>).

Hopefully they will help your use cases. Apologies if there's been any frustration, my goal wasn't to make a library that made someone's job harder.

An example usage:

dbCommand conn { cmdText "SELECT 1" }
|> Db.scalar Convert.ToInt32  // or, Db.Async.scalar Convert.ToInt32
|> printfn "%i" // outputs: 1

Once I get the thumbs up from you, I'll release a new minor version of the package to include this in the API.

@FoggyFinder
Copy link
Author

perfect, thank you

@pimbrouwers
Copy link
Owner

pimbrouwers commented Jul 4, 2021

You can test this out in 6.1.0-beta1.

@pimbrouwers pimbrouwers reopened this Jul 4, 2021
@FoggyFinder
Copy link
Author

Wonderful 🎉

During testing got a database is locked exception that wasn't handled inside of Donald. Not sure why yet. Probably I wasn't careful when rewrited some parts.

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

No branches or pull requests

2 participants