-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
support F# anonymous types #1233
base: main
Are you sure you want to change the base?
support F# anonymous types #1233
Conversation
65256db
to
a2d611c
Compare
done, @mgravell @NickCraver ready to review |
@mgravell anything i can help with for this PR? |
@NickCraver @mgravell any specific issue with this PR i need to address? sorry but anonymous types in f# are really handy with dapper, but are really bugged without this PR |
I totally understand. It isn't personal - I simply haven't had chance to look yet - I'm juggling time between multiple projects. The existence of tests in the PR is great. I'll try and look for the next drop. Sorry for delays. |
Np at all @mgravell , i know it may happen, it was just to check if there was something todo to help meanwhile i'll rebase it thanks a lot! |
d6d9d4f
to
c81f992
Compare
@enricosada did you see Marc's comment above? We'd love to get this in, but it could be a fair bit more efficient if intent matches. |
@NickCraver no, i missed that sry, let me check. thx for ping |
@mgravell i added some comments, because i not understood your feedback |
Hi @enricosada - I got nudged by a mutual friend to remind me to look at this again; sorry for delay. Can I check - the confusion we're talking about - is this the zip vs dual sort vs arrays thing? My understanding - and please do correct me if I'm wrong here - is that we have two arrays; one of the names, one of the types, matching by index; so if we have 3 columns, we might have names: "def", "ghi", "abc" as I read the code, the intent here is to sort by the names, preserving the respective positions in both arrays, so we end up with: names: "abc", "def", "ghi" is that correct? If so, that's exactly what the dual array version of Array.Sort does: string[] names = { "def", "ghi", "abc" };
Type[] types = { typeof(int), typeof(string), typeof(float) };
Array.Sort(names, types);
Console.WriteLine($"names: {string.Join(", ", names)}");
Console.WriteLine($"types: {string.Join(", ", types.Select(x => x.Name))}");
/* output:
names: abc, def, ghi
types: Single, Int32, String
*/ Have I misunderstood? |
hi @mgravell Maybe i didnt understood what the dual array version of Array.Sort does. I think that's exactly what i want, sort two arrays based on values of first array and maintaining the sync. Let me rebase and use Array.Sort |
to confirm, yes: that is an accurate description of what |
c81f992
to
9bf3072
Compare
9bf3072
to
c053dfc
Compare
Can I ask about the status of this? I would like to use this feature in my project and can clone this branch for now, but only if it is expected to make it into master. |
I just got hit by this as well. Would be awesome if it made it in a new release! |
@mgravell i think this is ready for review and merge. happy to do any change required |
I keep coming up against this issue. Is there a release schedule in mind? |
will re-review; sorry for dropping between cracks |
Code seems good; checking the tests locally for regressions - seems fine; you're happy that it is working from F#? |
@mgravell @enricosada The offending code is let parameters = Map ["ids", box ids; "dates", box dates]
use conn = new MySqlConnection(connectionString)
conn
|> dapperMapParametrizedQuery<{|id:int; date:DateTime; variance:float|}> sql parameters
|> Seq.groupBy (fun x -> x.date)
|> Seq.map (fun (dt, sq) -> dt, sq |> Seq.map (fun x -> {id = x.id; variance = x.variance})) |> Ok So either I have built it wrong, or am using it wrong. |
Let me check your repro
From mobile
…--
Enrico Sada
@enricosada
________________________________
Da: TravisLeithVeloqx <notifications@github.com>
Inviato: Thursday, April 16, 2020 8:59:59 AM
A: StackExchange/Dapper <Dapper@noreply.github.com>
Cc: Enrico Sada <enrico@sada.io>; Mention <mention@noreply.github.com>
Oggetto: Re: [StackExchange/Dapper] support F# anonymous types (#1233)
@mgravell<https://github.com/mgravell> @enricosada<https://github.com/enricosada>
Until now I have not actually attempted to use this.
Today I pulled this branch and built it. I added the Dapper.dll to my the project I am currently working on and attempted to use an anonymous record. I get the same parameterless default constructor error.
The offending code is
let parameters = Map ["ids", box ids; "dates", box dates]
use conn = new MySqlConnection(connectionString)
conn
|> dapperMapParametrizedQuery<{|id:int; date:DateTime; variance:float|}> sql parameters
|> Seq.groupBy (fun x -> x.date)
|> Seq.map (fun (dt, sq) -> dt, sq |> Seq.map (fun x -> {id = x.id; variance = x.variance})) |> Ok
So either I have built it wrong, or am using it wrong.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1233 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AABD6K5GVU7DEJWOPXLI77DRM2UG7ANCNFSM4HECYHLQ>.
|
@enricosada is this working? |
I updated this branch to latest main - still prepared to merge but we don't use F# and would really appreciate some community feedback on whether this works as intended. |
@NickCraver thanks a lot. I’ll check tomorrow morning |
@enricosada How can I test it? Should it work with unordered ctor parameters? |
pew, has been a while. Maybe I should take a look into that. |
So general suggestion from my side, I would have gone down the route and added another test folder with an F# project that depends on Dapper and uses it with anon records to make sure that the feature is functional. Anything against going that far? |
@NickCraver are you still open for having this change? If yes, could we reiterate on what you want to see from your side to be changed to get it merged, considering how long it has been since work has been done on this PR & topic? |
Support F# anonymous types in query like
To initialize the constructor argument correctly, i first tried to reoder the il fields doing the read from datareader to match constructor arguments position, not datareader name position, but i got:
so cannot be done like this, because read need to be sequential by column index.
To maintain the read order, before initializing the constructor, if needed, it will:
fix #1226