demetrixbio / FSharp.Data.Npgsql

F# type providers to support statically typed access to input parameters and result set of sql statement in idiomatic F# way. Data modifications via statically typed tables.
Apache License 2.0
127 stars 15 forks source link

Distinct Select queries projecting the same columns should have same Record type #54

Open Swoorup opened 4 years ago

Swoorup commented 4 years ago

Suppose I have the following:

        type SelectAll = NpgsqlCommand<"""
            SELECT name, address, state FROM person_table 
        """, TPConnStr>

        type SelectByState = NpgsqlCommand<"""
            SELECT name, address, state FROM person_table 
            WHERE state = @state
        """, TPConnStr>

Should both Selects result into the same type? Reason is that I am executing either one of the query based on a enum but have to write a minor boilerplate due to those types not having equality constraints.

Swoorup commented 4 years ago

As a workaround, I can fix this using Tuples.

daz10000 commented 4 years ago

I think unfortunately that since they are different queries they will generate distinct types even if they look structurally the same. You can use Fsharp structural type constraints to match against a record with members x Y and z to generalize the copying code.

Von meinem iPhone gesendet

Am 28.01.2020 um 4:20 PM schrieb Swoorup Joshi notifications@github.com:

 As a workaround, I can fix this using Tuples.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Swoorup commented 4 years ago

@daz10000 Does such a thing even exists in F#? You can only apply constraints for members in class I would have thought

kerams commented 4 years ago

That's an interesting suggestion, Daz. Unfortunately, doing something like http://www.fssnip.net/3z/title/Member-constraints-on-records turns out more verbose than just using the tuple result type (though you do get property name safety instead of relying on property positions in the tuple).

Swoorup commented 4 years ago

Is it possible to add anonymous records instead as an alternative projection? As anonymous records are structurally typed.

    let Aa () = {| A = "dsds" |}
    let Ba () = {| A = "asd"; B = "dssd" |}
    let Ca () = {| A = "asd"; B = "dssd" |}

    let aa = Aa() = Ba() // Invalid
    let ca = Ba() = Ca() // Valid
daz10000 commented 4 years ago

I haven’t looked into if that’s technically feasible. The whole point of the type provider is to provide the types and if you don’t get a named type back it’s hard to profess the results

Am 30.01.2020 um 7:41 PM schrieb Swoorup Joshi notifications@github.com:

 Is it possible to add anonymous records instead?

let Aa () = {| A = "dsds" |}
let Ba () = {| A = "asd"; B = "dssd" |}
let Ca () = {| A = "asd"; B = "dssd" |}

let aa = Aa() = Ba() // Invalid
let ca = Ba() = Ca() // Valid

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

kerams commented 4 years ago

Digging around while working on #55, I think I've found a way how this might be doable for NpgsqlConnection (but not NpgsqlCommand because each of those is a separate root type). Instead of declaring provided types from individual CreateCommand on the commands themselves (like DvdRental.Commands.``CreateCommand,CommandText"SELECT 4 as num, title from film"``.Record), they'd need to go directly under the root type (like DvdRental.Record) -- this appears to work. Then you'd obviously need to encode the type structure in the type name (maybe something like DvdRental.``num:Option<int>*title:string`` or a hash thereof, because the name could get very long), so that you can easily decide if a new CreateCommand should reuse an existing provided type on NpgsqlConnection (do property names and types all match? - positions in the select could be irrelevant, which I'd consider a bonus) -- this I haven't tried yet..

daz10000 commented 4 years ago

A hash type sounds efficient. Identical return structures get identical hashes and so there is implicit reuse.

Von meinem iPhone gesendet

Am 02.02.2020 um 12:23 PM schrieb kerams notifications@github.com:

 Digging around while working on #55, I think I've found a way how this might be doable for NpgsqlConnection (but not NpgsqlCommand because each of those is a separate root type). Instead of declaring provided types from individual CreateCommand on the commands themselves (like DvdRental.Commands.CreateCommand,CommandText"SELECT * from film".Record), they'd need to go directly under the root type (like DvdRental.Record) -- this appears to work. Then you'd obviously need to encode the type structure in the type name (maybe something like DvdRental.num:Option<int>*name:string or a hash thereof, because the name could get very long), so that you can easily decide if a new CreateCommand should reuse an existing provided type on NpgsqlConnection -- this I haven't tried yet..

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

kerams commented 4 years ago

devenv_8b9oOAoRmN

Surprisingly trivial to implement the most basic version.

At the same time, this unfortunately does not compile because CreateCommand needs to add the type first.

let cmp (v1: DvdRental.``id:Int32, w:String``) (v2: DvdRental.``id:Int32, w:String``) =
    if v1.id = v2.id && v1.w = v2.w then
        printfn "liftoff"
    else
        printfn ":/"

use cmd = DvdRental.CreateCommand<"select id, 'dsad' as w from table_with_composites ">(dvdRental)
let res = cmd.Execute().Head

use cmd2 = DvdRental.CreateCommand<"select 'dsad' as w, id from table_with_composites where id > 0">(dvdRental)
let res2 = cmd2.Execute().Head

cmp res res2
Swoorup commented 4 years ago

Back to producing anonymous records instead, I don't really see what is the issue with it? It solves column order and this problem too. You can always alias the type if you want can't you?

kerams commented 4 years ago

Briefly put, the issue concerns the question whether it's technically possible to send and use an anonymous record across the TP <-> Visual Studio boundary. Should it work, it would be better than what I proposed above in many respects. I'm preparing a PR for this and will make sure to check anonymous records too.

kerams commented 4 years ago

Yeah, I think this is a non-starter. Look at https://github.com/demetrixbio/FSharp.Data.Npgsql/blob/master/src/DesignTime/QuotationsFactory.fs#L223

The compiler creates anonymous records on demand when you construct an instance of it in code. Here however, you have a list of properties the record should have, but they'd need to be known at compile time (compile time of the TP, not the application using it) in order to get a reference to the Type of the anonymous record (and the only way to construct it as far as I can see is use its 'literal definition', like typeof<{| a: int |}>) to pass to VS/compiler,

Swoorup commented 4 years ago

Ah that makes sense. Compile-time programming in F# is painfully limiting. :/

Swoorup commented 4 years ago

However, would your solution be workable also in NpgsqlCommand? Although the root type can be different, the type returned when execute() is called could be the same? Is there a branch you have I could currently try to hack (No experience in TPs though)

kerams commented 4 years ago

It would not. Every time you new up NpgsqlCommand, it creates a new instance of the TP with a separate provided type cache, so you cannot reuse the same types, because there is no way to share the Type instance you're providing. Why are you not using NpgsqlConnection though? I'd strongly urge you to since I consider the former pretty much obsolete with VS 2017+.

There is no branch yet, but I could open a PR later today.

Swoorup commented 4 years ago

Resolved in https://github.com/demetrixbio/FSharp.Data.Npgsql/pull/72

Nice job btw

When can we expect a new nuget package to be released?

kerams commented 4 years ago

@Swoorup, it's out now. Let me know if you have any thoughts on it.

Swoorup commented 4 years ago

@kerams Been using this for a while now. Quite nice feature. The only downside I am facing is that the type name can get quite long and if you are returning a slightly different type, it can be bit of trial and error to figure which type you are looking for.

image

A remedy to this can I suggest, can be adding a optional parameter to name the type? So, elsewhere returning the same type will have the same name reference too. If it matches, its the same type.

I have currently been aliasing the type where I am using it, so not too much fussed but just something nice to have.

Swoorup commented 4 years ago

Let me know if I should move it to a new ticket instead.

kerams commented 4 years ago

Yeah, I expected it could get gnarly like this. Optionally naming the type would have at least one major issue though. Because types for commands are processed sequentially top to bottom in a file, you'd need to make sure to name the type in the first CreateCommand that returns it (that's where the type is first generated (named) and cached (this is also the reason why you cannot refer to a type "above" its first occurence)). Naming it anywhere else, or multiple times, would seemingly do nothing because it's already been used and cached without an explicit name:

use cmd = DvdRental.CreateCommand<"select id from table">(dvdRental)
let res = cmd.Execute().Head

use cmd2 = DvdRental.CreateCommand<"select id from table where id > 0", TypeName = "TT">(dvdRental)
let res2 = cmd2.Execute().Head

// ok
let cmp (v1: DvdRental.``id:Int32``) (v2: DvdRental.``id:Int32``) =
    if v1.id = v2.id then
        printfn "liftoff"
    else
        printfn ":/"

// would not compile
let cmp' (v1: DvdRental.TT) (v2: DvdRental.TT) =
    if v1.id = v2.id then
        printfn "liftoff"
    else
        printfn ":/"

Something that I could do to ease the pain a little is generate an Xml doc summary for each type containing a short hash of the name. You'd still need to hover over a type to see and compare it to what you're looking for, but it might be the easier option when you can't even see the entire name at a glance.

Swoorup commented 4 years ago

Regarding naming ordering, yeah I did expect that. Thinking wildly here, maybe explicitly naming could be its own seperate type and unnamed could fallback to the hash type names with XML doc summary?

kerams commented 4 years ago

Yeah, that's doable. While structurally equivalent, they'd be incompatible.