JordanMarr / SqlHydra

SqlHydra is a suite of NuGet packages for working with databases in F# including code generation tools and query expressions.
MIT License
223 stars 22 forks source link

Support net8.0 #73

Closed EverybodyKurts closed 8 months ago

EverybodyKurts commented 10 months ago

Currently the project supports net6.0 and net7.0. It'd be great if it could support net8.0.

JordanMarr commented 10 months ago

My understanding is that it .NET 8 should be able to run it as-is. (I assume you are referring to the SqlHydra.Cli tool). Although, I haven't actually tried it to verify.

EverybodyKurts commented 10 months ago

I'm running this command inside the mcr.microsoft.com/vscode/devcontainers/dotnet:8.0-bookworm-slim docker image:

$ dotnet sqlhydra npgsql
You must install or update .NET to run this application.

App: /root/.nuget/packages/sqlhydra.cli/2.2.0/tools/net7.0/any/SqlHydra.Cli.dll
Architecture: x64
Framework: 'Microsoft.NETCore.App', version '7.0.0' (x64)
.NET location: /usr/share/dotnet/

The following frameworks were found:
  8.0.0 at [/usr/share/dotnet/shared/Microsoft.NETCore.App]

Learn more:
https://aka.ms/dotnet/app-launch-failed

To install missing framework, download:
https://aka.ms/dotnet-core-applaunch?framework=Microsoft.NETCore.App&framework_version=7.0.0&arch=x64&rid=linux-x64&os=debian.12
JordanMarr commented 10 months ago

Good to know! I’ll get it out today.

JordanMarr commented 10 months ago

I always forget how big of a task this is. (It definitely won't be done tonight. 😴)

EverybodyKurts commented 10 months ago

It is a big task and why I wanted to try to fork this, do it myself, and issue a pull request. Then that led me to thinking that this project could come with a docker devcontainer image that has all the dependencies and build tasks baked in (getting fake set up properly would be huge).

JordanMarr commented 10 months ago

I just tried to update the devcontainer stuff to the latest .net 8 image and fixed the devcontainer.json settings file. All the containers pulled down when I launch vs code with the devcontainers extension, but it fails everytime with: "An error occurred setting up the container."

TBH, it's always been easier to just go to the .devcontainer folder and run docker-compose up. Yeah, you have to have .NET 8 installed on your local machine, but you obviously have that anyway.

JordanMarr commented 10 months ago

Taking a stab at .NET 8 for SqlHydra.Cli. I think I can have this released tonight! I'm so glad I moved to NUnit a few weeks ago. It's sooo much faster and easier to run one-off tests!

I don't think SqlHydra.Query needs to be updated.

JordanMarr commented 10 months ago

The Npgsql enum tests are failing in net8 during insert.

If I don't generate a ProviderDbType for the mood enum, no NpgsqlDbType parameter type is assigned.

    [<CLIMutable>]
    type person =
        { [<SqlHydra.ProviderDbType("Text")>]
          name: string
          currentmood: mood }

This worked fine for net6/net7, but now fails with: "Writing values of 'Npgsql.AdventureWorksNet8.ext+mood' is not supported for parameters having no NpgsqlDbType or DataTypeName. Try setting one of these values to the expected database type.."

I've tried with a ProviderDbType of "Text" and "Unknown", but those both fail as well: "Writing values of 'Npgsql.AdventureWorksNet8.ext+mood' is not supported for parameters having NpgsqlDbType 'Text'."

Let me know if you can manually insert an enum parameter using Npgsql 8.

JordanMarr commented 10 months ago

I was able to get the tests to pass for .NET 8 if I downgraded to Npgsql v7. So, perhaps it's worth pushing a new version as this seems to be a Npgsql issue.

JordanMarr commented 10 months ago

Released with support for .NET 8. https://github.com/JordanMarr/SqlHydra/releases/tag/cli-v2.3.0

EverybodyKurts commented 10 months ago

Damn, even when I downgrade npgsl to 7.0, I still get the is not supported for parameters having no NpgsqlDbType or DataTypeName. Try setting one of these values to the expected database type..) error.

JordanMarr commented 10 months ago

Hmm.. I was definitely able to get the integration test to work with Npgsql v7.

https://github.com/JordanMarr/SqlHydra/blob/7b98ed435efd49a40be14b33bad6400194503d3d/src/Tests/Npgsql/QueryIntegrationTests.fs#L562-L566

JordanMarr commented 10 months ago

Any update on your issue?

kurt-mueller-osumc commented 10 months ago

npgsql is versioned at 7... no joy :(

I have yet to take a look at the tests. I'll do that now :)

EverybodyKurts commented 10 months ago

Ok, the following works when I use insertTask instead of insertAsync:

#r "nuget: SqlHydra.Query"
#r "nuget: Npgsql, 7.0.6"

#r "./src/Reports/bin/Debug/net8.0/Reports.dll"

open System
open System.Threading.Tasks

open SqlHydra.Query
open SqlHydra.Query.NpgsqlExtensions
open Npgsql

// the database schema generated by SqlHydra.Cli
open GoSeq.ETL.Reports.DbSchema

// the connection string for my local docker postgres instance
let connectionString = "Host=db;Username=goseq;Password=groin-spoiling-juggling-alone;Database=goseq"

let openContext () =
    let compiler = SqlKata.Compilers.PostgresCompiler()
    let conn = new NpgsqlConnection(connectionString)

    conn.Open()

    new QueryContext(conn, compiler)

let (sample: ``public``.samples) = {
    id = "sample-id"
    category = ``public``.sample_categories.normal
    biopsy_site = Some "biopsy-site"
    sample_type = Some "sample-type"
    sample_reports_count = 0
    created_at = DateTime.UtcNow
    updated_at = DateTime.UtcNow
}

let upsertSample () : int Task =
    use ctx = openContext ()

    let dataSourceBuilder = NpgsqlDataSourceBuilder(connectionString)

    dataSourceBuilder.MapEnum<``public``.sample_categories>() |> ignore

    insertTask (Create (fun _ -> ctx)) {
        for existingSample in table<``public``.samples> do
        entity sample
        excludeColumn sample.sample_reports_count
        onConflictDoUpdate existingSample.id (
            existingSample.sample_type,
            existingSample.biopsy_site,
            existingSample.category,
            existingSample.updated_at
        )
    }

let upsertions = upsertSample () |> Async.AwaitTask |> Async.RunSynchronously

upsertions
EverybodyKurts commented 10 months ago

The following snippet crashes when I use insertAsync.

#r "nuget: SqlHydra.Query"
#r "nuget: Npgsql, 7.0.6"

#r "./src/Reports/bin/Debug/net8.0/Reports.dll"

open System
open System.Threading.Tasks

open SqlHydra.Query
open SqlHydra.Query.NpgsqlExtensions
open Npgsql

// the database schema generated by SqlHydra.Cli
open GoSeq.ETL.Reports.DbSchema

// the connection string for my local docker postgres instance
let connectionString = "Host=db;Username=goseq;Password=groin-spoiling-juggling-alone;Database=goseq"

let openContext () =
    let compiler = SqlKata.Compilers.PostgresCompiler()
    let conn = new NpgsqlConnection(connectionString)

    conn.Open()

    new QueryContext(conn, compiler)

let (sample: ``public``.samples) = {
    id = "sample-id"
    category = ``public``.sample_categories.normal
    biopsy_site = Some "biopsy-site"
    sample_type = Some "sample-type"
    sample_reports_count = 0
    created_at = DateTime.UtcNow
    updated_at = DateTime.UtcNow
}

let upsertSample () : int Async =
    use ctx = openContext ()

    let dataSourceBuilder = NpgsqlDataSourceBuilder(connectionString)

    dataSourceBuilder.MapEnum<``public``.sample_categories>() |> ignore

    insertAsync (Create (fun _ -> ctx)) {
        for existingSample in table<``public``.samples> do
        entity sample
        excludeColumn sample.sample_reports_count
        onConflictDoUpdate existingSample.id (
            existingSample.sample_type,
            existingSample.biopsy_site,
            existingSample.category,
            existingSample.updated_at
        )
    }

let upsertions = upsertSample () |> Async.RunSynchronously

upsertions

I get this error:

System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'NpgsqlConnection'.
   at Npgsql.NpgsqlConnection.Open(Boolean async, CancellationToken cancellationToken)
   at Npgsql.NpgsqlConnection.Open()
   at SqlHydra.Query.SelectBuilders.ContextUtils.tryOpen(QueryContext ctx)
   at SqlHydra.Query.SelectBuilders.ContextUtils.getContext(ContextType ct)
   at SqlHydra.Query.InsertBuilders.Run@95-23.Invoke(Unit unitVar)
   at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvoke[T,TResult](AsyncActivation`1 ctxt, TResult result1, FSharpFunc`2 part2) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 510
   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 112
--- End of stack trace from previous location ---
   at Microsoft.FSharp.Control.AsyncResult`1.Commit() in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 454
   at Microsoft.FSharp.Control.AsyncPrimitives.QueueAsyncAndWaitForResultSynchronously[a](CancellationToken token, FSharpAsync`1 computation, FSharpOption`1 timeout) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 1140
   at Microsoft.FSharp.Control.AsyncPrimitives.RunSynchronously[T](CancellationToken cancellationToken, FSharpAsync`1 computation, FSharpOption`1 timeout) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 1167
   at Microsoft.FSharp.Control.FSharpAsync.RunSynchronously[T](FSharpAsync`1 computation, FSharpOption`1 timeout, FSharpOption`1 cancellationToken) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 1511
   at <StartupCode$FSI_0003>.$FSI_0003.main@() in /workspace/etl/scratch.fsx:line 97
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
JordanMarr commented 10 months ago

Instead of manually creating the ctx, you should let SqlHydra manage creating and disposing the context for you:

let upsertSample () : int Async =
    // use ctx = openContext () // Remove this

    let dataSourceBuilder = NpgsqlDataSourceBuilder(connectionString)

    dataSourceBuilder.MapEnum<``public``.sample_categories>() |> ignore

    insertAsync (Create openContext) {
        for existingSample in table<``public``.samples> do
        entity sample
        excludeColumn sample.sample_reports_count
        onConflictDoUpdate existingSample.id (
            existingSample.sample_type,
            existingSample.biopsy_site,
            existingSample.category,
            existingSample.updated_at
        )
    }

If you want to explicitly manage the ctx creation and disposal yourself, you will need to wrap your entire method in an async CE to prevent the ctx from being disposed prematurely:

let upsertSample () : int Async = async {
    use ctx = openContext ()

    let dataSourceBuilder = NpgsqlDataSourceBuilder(connectionString)

    dataSourceBuilder.MapEnum<``public``.sample_categories>() |> ignore

    insertAsync (Shared ctx) {
        for existingSample in table<``public``.samples> do
        entity sample
        excludeColumn sample.sample_reports_count
        onConflictDoUpdate existingSample.id (
            existingSample.sample_type,
            existingSample.biopsy_site,
            existingSample.category,
            existingSample.updated_at
        )
    }
}

In fact, the error you received is the reason I added Create: to avoid having to manually wrap single queries in an async or task CE.