pimbrouwers / donald Goto Github PK

Hi, I seem to be running in to an issue when using Db.Async.exec where it seems to be throwing an exception when it encounters an error instead of returning the error in a Result. I have created a repo here: https://github.com/Darkle/donald-async-issue and if you run dotnet restore then sqlite3 foo.db ".read ./init-db.sql" to create the sqlite db, then dotnet run you should get the following output:

dbResult Ok ()
DB Error: Microsoft.Data.Sqlite.SqliteException (0x80004005): SQLite Error 1: 'no such table: Asd'.
   at Microsoft.Data.Sqlite.SqliteException.ThrowExceptionForRC(Int32 rc, sqlite3 db)
   at Microsoft.Data.Sqlite.SqliteCommand.PrepareAndEnumerateStatements(Stopwatch timer)+MoveNext()
   at Microsoft.Data.Sqlite.SqliteCommand.GetStatements(Stopwatch timer)+MoveNext()
   at Microsoft.Data.Sqlite.SqliteDataReader.NextResult()
   at Microsoft.Data.Sqlite.SqliteCommand.ExecuteReader(CommandBehavior behavior)
   at Microsoft.Data.Sqlite.SqliteCommand.ExecuteReader()
   at Microsoft.Data.Sqlite.SqliteCommand.ExecuteNonQuery()
   at System.Data.Common.DbCommand.ExecuteNonQueryAsync(CancellationToken cancellationToken)
--- End of stack trace from previous location ---
   at [email protected]() in C:\Users\pim\git\Donald\src\Donald\Db.fs:line 119
   at [email protected]() in C:\Users\pim\git\Donald\src\Donald\Db.fs:line 110
   at [email protected]() in C:\Users\pim\git\Donald\src\Donald\Db.fs:line 113
   at Program.Pipe #4 input at line [email protected]() in /home/coop/Coding/scratch/donald-async-issue/Program.fs:line 34

You can see that the first one worked and the result is a Result of Ok unit, but when we do the second db call with a table that doesnt exist, Db.Async.exec throws instead of returning the error in a Result.

Every time I go here, I read about this mysterious @Dysme, and I'm thinking should I bother to raise an issue or not. Sooner or later I'm not going to resist the temptation.

Thank you for a wonderful F# library.

Hello!
I'm pretty new to the F# world, and started to write an application using Falco with Donald. I was following the setup of FalcoJournal for my application, however when I came to DB access, I kept getting a sequential access error.

module StateProvider = 
    type GetAll = unit -> DbResult<State list>
    let getAll (log : ILogger) (conn : IDbConnection) : GetAll =
        fun () ->
            let sql = "SELECT state, state_abbreviation FROM states;"

            let fromDataReader (rd : IDataReader) = 
                {StateAbbreviation = rd.ReadString "state_abbreviation"; State = rd.ReadString "state"}

            dbCommand conn { cmdText sql }
            |> Db.query fromDataReader
            |> DbResult.logError log

When I downgraded to version 5, I was able to get the code to work (change Db.query to DbConn.query)

I noticed that 5.0.* did not have sequential access turned on, which is why I assume it's working when I downgrade. What would I need to update in this code for it to work with sequantial access turned on?

Thank you in advance, your work is much appreciated!

SqlClient supports a native DateTimeOffset type but it looks like Donald assumes the value will be stored as a string. The following line shows how it is currently working.

Therefore when using the data type DateTimeOffset you get the following exception:

Exception thrown: 'System.InvalidCastException' in System.Data.SqlClient.dll
Unable to cast object of type 'System.DateTimeOffset' to type 'System.String'.

What's the best way to read a DateTimeOffset with Donald, would we need a database specific assembly to read database specific types?

At the very least it might be worth removing ReadDateTimeOffset if it causes exceptions depending on the database.

Proposed API:

let queryAsync sql param conn = ... -> Task<'a list>
let querySingleAsync sql param conn = ... -> Task<'a>
let scalarAsync sql param conn = ... -> Task<'a>
let execAsync sql param conn = ... -> Task<unit>
let tranQueryAsync  sql param tran = ... -> Task<'a list>
let tranQuerySingleAsync sql param tran = ... -> Task<'a>
let tranScalarAsync sql param tran = ... -> Task<'a>
let tranExecAsync sql param tran = ... -> Task<unit>

Can we add an extension to produce the sql, the parameters, the reader, etc. from reflection for the basic insert, select, update operations?

@pimbrouwers,
Hey, thanks for this lightweight ADO.Net wrapper library.

Do you have any plan to support the transaction in Fluent syntax?
I mostly prefer Fluent syntax and currently there is no any method to pass transaction to the Db.newCommand method.

I have to port an app to .NET that connects to a third-party database. I generally have very limited access to the database, given only a view of the data and permission to fetch from that view. Unfortunately, when I used Donald.DbConn.Async.query, my request was rejected. After confirming everything else, I found that explicitly not using a transaction was the only way the data came back successfully, so it seems that I don't have permission to use those. And when I looked around in the code, there didn't appear to be any way to not use a transaction.

Working around it wasn't too bad...

    let getThings () =
        use conn = createConn()
        use cmd = 
            dbCommand conn {
                cmdType CommandType.Text
                cmdText "
                    select * from things
                "
                cmdParam [
                ]
            }
        
        //  DbConn.Async.query Thing.fromReader cmd

        conn.Open()
        use reader = cmd.ExecuteReader()
        let listData =
            [
                while reader.Read() do
                yield Thing.fromReader reader
            ]
        listData

...and since it can be "fixed" on my end with just a helper function at no cost to performance, I'm not especially worried about it. But, I figured I'd report it since it was a pain point in the Donald experience (albeit a very small one). I think any attempt to make use of transactions configurable would likely lead to code duplication and/or overcomplication, so I'm not sure any changes are even worth it. If you happen to have any ideas though, then by all means. Otherwise, you can just close this as a wontfix. ;-)

At my work, I've developed a similar lightweight framework where we pretty much only ever use stored procedures in our code. I'm wanting to make the move to an open source framework, and yours is the most promising, but we would prefer to be able to set the command type accordingly.

I envision that this could be done by creating some basic private functions that accept it as an argument and then reuse existing functions / add a new one to handle the versions that supply the extra parameter. Ex:

/// Create a new IDbCommand  
let newIDbCommand (sql : string) (tran : IDbTransaction) (cmdType : CommandType) =
    let cmd = tran.Connection.CreateCommand()
    cmd.CommandType <- cmdType
    cmd.CommandText <- sql
    cmd.Transaction <- tran
    cmd 

...

/// Create a new IDbCommand  
let newCommand (sql : string) (dbParams : DbParam list) (tran : IDbTransaction) (cmdType : CommandType) =
    let cmd = newIDbCommand sql tran cmdType 
    assignDbParams cmd dbParams
    cmd

...

let private tranOp (sql : string) (param : DbParam list) (map : IDataReader -> 'a) (tran : IDbTransaction) (cmdType : CommandType) =
    use cmd = newCommand sql param tran cmdType
    use rd = cmd.ExecuteReader()
    let results = [ while rd.Read() do yield map rd ]
    rd.Close() |> ignore
    results

/// Query for multiple results within transaction scope
let tranQuery (sql : string) (param : DbParam list) (map : IDataReader -> 'a) (tran : IDbTransaction) =
    tranOp sql param map tran CommandType.Text

///  for multiple results within transaction scope
let tranSproc (sql : string) (param : DbParam list) (map : IDataReader -> 'a) (tran : IDbTransaction) =
    tranOp sql param map tran CommandType.StoredProcedure

This then keeps the API consistent for things like tranQuery so as to not break compatibility too much.

I was gonna go ahead and work on that to submit a PR, but wanted to consult with you first to see if you even liked the implementation concept.

Hi @pimbrouwers, thanks for this amazing library. I was really looking for such kind of lightweight ADO.Net wrapper library.
I was doing some experiment with the library and notice that whenever I use Db.Async.query, I couldn't find the proper way to manage the Task-Result returned by the query. Actually, the async & task is always confusing for me.

This is not actually a bug and but just want some example of using Db.Async.query with SQL Server and handling the output to chain in pipeline of Async<Result<'a, DbError>>.

Actually, I tried with Async.AwaitTask and then Async.RunSynchronously but then started getting random error of "Invalid operations: the connection is closed" or "Invalid Operations: the data reader is closed.".
The reason of these errors might be because of the Async.query has already executed and the connection and datareader have been already closed, but the RunSynchronously is trying to execute method and fetch data.

There are some open issues in Microsoft.Data.SqlClient regarding the Async usage as well, but what I observe is, currently I am getting the errors because of my implementation of code, and once I will be able to execute code successfully then might be started to facing those issues.

After some extended time using the library, it's become apparent to me that the type alias atop Result<'a, DbExecutionError> is providing little to no value. Aside from the source code type annotations being slightly less verbose.

As such, I'm proposing that they be removed. I intend to preserve the dbResult {} and dbResultTask {} expressions, though I could easily be convinced to drop them now that we're relying on a more generic type.

open Donald

/* 
 * SqlType qualified access */
 
// New form
let sqlTypeInt = SqlType.Int 1

// Compiler error
let sqlTypeInt = Int 1 

/* 
 * DbResult & DbResultTask */

// New form
let readAuthor : Result<Author, DbExecutionError> = // ..
let readAuthor : Task<Result<Author, DbExecutionError>> = // ..

// Compiler error
let readAuthor : DbResult<Author> = // ...
let readAuthor : DbResultTask<Author> = // ...

// Still works (for now)
dbResult {}
dbResultTask {}

I'm using Donald with NpgSQL and find out that the Db.Async.execMany is throwing exception due to parallel execution of the commands. It's not occurring every time, probably depends on execution time of the queries.
The issue is probably not reproducible with SQLite (maybe it allows multiple command on the same connection in parallel).
I found the issue with NpgSQL but I'm 99% sure the behaviour is the same with SQLServer.

Issue is coming from this line: https://github.com/pimbrouwers/Donald/blob/master/src/Donald/Db.fs#L110
I cannot think of an easy way to fix it without adding task CE (ply or TaskBuilder). None CE dependent solution should probably rely on combining continuation recursively.
I would gladly help to apply a fix for it :)

Exception:
Npgsql.NpgsqlOperationInProgressException (0x80004005): A command is already in progress: INSERT INTO ingredient_months (ingredient_id, month_id) VALUES (@ingredient_id, @month_id)

Failing code:
do!
conn
|> Db.newCommand "INSERT INTO ingredient_months (ingredient_id, month_id)
VALUES (@ingredient_id, @month_id)"
|> Db.setTransaction tran
|> Db.Async.execMany months
|> TaskResult.mapError (FailedExecutionError >> raise)

Workaround:
for m in months do
do!
conn
|> Db.newCommand "INSERT INTO ingredient_months (ingredient_id, month_id)
VALUES (@ingredient_id, @month_id)"
|> Db.setParams m
|> Db.setTransaction tran
|> Db.Async.exec
|> TaskResult.mapError (FailedExecutionError >> raise)

btw: nice library, it's super pleasant to write infra code with it :)

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.

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

Hi Pim,

I'm wondering what SQL type I should use for a SQLite JSON column. Do I just need SqlType.String like this (raw is the JSON field):

module Command = 

    let insertRawOrder orderId jsonString = {
        Sql = "INSERT INTO purchase (id, raw) VALUES (@id, @raw);"
        Params = [ 
            ("id",  SqlType.String orderId) 
            ("raw", SqlType.String jsonString) // SQLite JSON field
        ] 
    }

Hey there!

Problem statement

I started playing with this library a bit more in depth and one of the things missing for me is CancellationToken support. The reason passing down a CancellationToken so database connections don't stay open longer than they need for.

For example, a user could create a complex search, start the query, then decide to do something else, and move away from the page. Without passing along the CancellationToken to the database query, it may run in the background for some time, consuming resources.

Changes

I'll be sending a Pull Request shortly with the proposed changes.

I want to add a sample of using Donald with the new SqlHydra generated data readers because the two seem like a great fit.

However, I ran into a problem that is related to the fact that Donald uses CommandBehavior.SequentialAccess by default. I think the issue is that the generated readers may, in some scenarios, try to read twice from the same reader ordinal, which SequentialAccess does not allow. It would be great if this wasn't the default, or if it could be changed.

Another issue (minor) is that Donald always returns a vanilla IDataReader, whereas SqlHydra can generate using a more specific reader (which can be useful in scenarios like where SQL Server has a custom reader for DateTimeOffset). This requires that the IDataReader passed by Db.query has to be manually casted to the provider specific reader generated by SqlHydra.

Changing this so that Donald can return a more specific data reader (based on the connection) could be beneficial for Donald in general, as well as making interop easier with SqlHydra. I think you may be able to do this by making Db.query take a generic type type arg for the data reader that implements a generic constraint that forces it to implement IDataReader.

For example, a RequiredColumn in SqlHydra can take an IDataReader or anything that implements it, like Microsoft.Data.SqlClient.SqlDataReader.

type RequiredColumn<'T, 'TReader when 'TReader :> System.Data.IDataReader>(reader: 'TReader, getter: int -> 'T, column) =
        inherit Column(reader, column)
        member __.Read(?alias) = alias |> Option.defaultValue __.Name |> reader.GetOrdinal |> getter

Hi, I am using Donald with Postgres, where I have a Logs table with one of the columns being a postgres enum. I'm not too sure how to read this properly with Donald.

Here is the sql:

CREATE TYPE "LogLevel" AS ENUM ('Trace', 'Debug', 'Information', 'Warning', 'Error', 'Critical');

CREATE TABLE "Logs" (
    "uniqueId" UUID NOT NULL DEFAULT gen_random_uuid(),
    "createdAt" DOUBLE PRECISION NOT NULL,
    "level" "LogLevel" NOT NULL,
    "message" TEXT,
    "service" TEXT,
    "error" TEXT,
    "other" TEXT,

    CONSTRAINT "Logs_pkey" PRIMARY KEY ("uniqueId")
);

Here is my F# type for a Log:

module RidoModels

type LogLevel =
  | Trace = 1
  | Debug = 2
  | Information = 3
  | Warning = 4
  | Error = 5
  | Critical = 6

[<CLIMutable>]
type Log =
  { uniqueId: string
    createdAt: double
    level: LogLevel
    message: Option<string>
    service: Option<string>
    error: Option<string>
    other: Option<string> }

Here is my F# DB code:

module DB.DB

open System.Data
open Donald
open Npgsql
open System

let conn =
  new NpgsqlConnection(Environment.GetEnvironmentVariable("DOTNET_DB_CONNECTION_STRING"))
  :> IDbConnection

module Log =
  let ofDataReader (rd: IDataReader) : RidoModels.Log =
    { uniqueId = rd.ReadString "uniqueId"
      createdAt = rd.ReadDouble "createdAt"
      level = rd.ReadString "level"
      message = rd.ReadStringOption "message"
      service = rd.ReadStringOption "service"
      error = rd.ReadStringOption "error"
      other = rd.ReadStringOption "other" }

The compiler complains about a type mismach for the level = rd.ReadString "level" line:

Type constraint mismatch. The type 
    'string'    
is not compatible with type
    'RidoModels.LogLevel' 

So how would I go about dealing with an DB enum in Donald?

Hi @pimbrouwers , I'm new to F# and .Net, and I've written helper classe t work with Donald, can you review it please?
https://github.com/arthur-s/FsDto

In the README, the signature of GetDateTimeOffset is string -> DateTime but I think it’s actually string -> DateTimeOffset. I think the nullable and option variants say this as well.

FYI, it looks like the newSproc function you made never updated its doc comment to say CommandType.StoredProcedure. Here.

The article Introducing the new Microsoft.Data.SqlClient describes Microsoft.Data.SqlClient as the way forward for ADO.NET.

I propose this library either support or transition to Microsoft.Data.SqlClient

For example, FsSql.Core library has this feature that is handy when there're many queries in a single transaction.

After spending some time reviewing the API, it dawned on me that a significant opportunity of reliability was being missed. That being, during param creation.

In order to implement a higher-level of constraint during parameter creation a DU was created:

type SqlType =
    | String     of String
    | AnsiString of String
    | Boolean    of Boolean
    | Byte       of Byte
    | Char       of Char
    | AnsiChar   of Char
    | DateTime   of DateTime
    | Decimal    of Decimal
    | Double     of Double
    | Float      of float
    | Guid       of Guid
    | Int16      of Int16
    | Int32      of Int32
    | Int        of int32
    | Int64      of Int64
    | Bytes      of Byte[]

The newParam function, now expects an instance of this:

exec
    "INSERT INTO author (full_name) VALUES (@full_name);"
    [ newParam "full_name" (SqlType.String "Jane Doe") ] // explicit type
    conn