JordanMarr / SqlHydra

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

`deleteAll` failing with SqlHydra.Query 1.1.0 #46

Closed MangelMaxime closed 1 year ago

MangelMaxime commented 1 year ago

Using the following code with SqlHydra.Query 1.1.0 result in an error at runtime:

module Tables =

    let encounterForms = table<dbo.EncounterForms>

    let! _ =
        deleteAsync (Shared ctx) {
            for _ in Tables.encounterForms do
                deleteAll
        }
System.NotImplementedException: The method or operation is not implemented.
   at SqlHydra.Query.QuotationVisitor.visit@10(FSharpExpr expr)
   at SqlHydra.Query.QuotationVisitor.visitFor[T](FSharpExpr`1 f)
   at SqlHydra.Query.DeleteBuilders.DeleteBuilder`1.For[T](QuerySource`1 state, FSharpExpr`1 forExpr)
   at Antidote.Server.Tests.Database.clean@57-1.Invoke(QueryContext _arg1) in /Users/mmangel/Workspaces/Github/Antidote-AI/antidote-apps/Server/Antidote.Server.Tests/Database.fs:line 73
   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

Downgrading to SqlHydra 1.0.5 with the following code:

module Tables =

    let encounterForms = table<dbo.EncounterForms> |> inSchema (nameof dbo)

    let! _ =
        deleteAsync (Shared ctx) {
            for _ in Tables.encounterForms do
                deleteAll
        }

works.

JordanMarr commented 1 year ago

Ok, I think that should be a simple fix.

MangelMaxime commented 1 year ago

In case it helps here is the configuration file I am using:

[general]
connection = "Server=127.0.0.1,33300;Database=XXX;User Id=SA;Password=XXXXXXXX;TrustServerCertificate=True"
output = "Server/Antidote.Database/Dbos.fs"
namespace = "Antidote.Database"
cli_mutable = false
[readers]
reader_type = "Microsoft.Data.SqlClient.SqlDataReader"
[filters]
include = []
exclude = []

With sqlhydra.sqlserver@1.1.1

JordanMarr commented 1 year ago

It seems to be caused by using the _ assignment. My first attempt to recreate it, I assigned it to an actual binding/variable and it works:

    for e in Tables.encounterForms do
        deleteAll

For deletes, I think your usage of _ makes sense when deleting all rows in a table, and so I am fixing this bug now.

For updates and inserts, I think I will throw a _ is unsupported exception since a type is required in order to get ProviderDbType column attributes.

For selects, it does not apply because it is not possible to use an underscore anyway as there is no selectAll keyword. (You would have to do select p for that.) Plus, it is required to assign to a projection binding anyway for selects.

JordanMarr commented 1 year ago

A fix has been pushed to the main branch that allows _ to be used only for DeleteBuilder queries.

MangelMaxime commented 1 year ago

Thank you @JordanMarr

I never tough it would be because of the _ 😅.

I will test it when released.

JordanMarr commented 1 year ago

Previous versions always created queries using the fully qualified table and column names, but a recent update changed it so that everything uses table aliases named after the actual table binding.

For example, this query:

for p in dbo.Tables.Products do 
select p

will now use p as the table alias:

SELECT p.*
FROM dbo.Products AS p

This change makes the generated queries nicer, but more importantly, it allows for same table joins. So that's what caused the odd bug!

SqlHydra.Query v1.1.1 is now released with the patch.

MangelMaxime commented 1 year ago

The alias makes senses.

Fix is also working for my project thank you very much.