I think I just imagined a way to implement multi column joins in the Linq API that might work... 🤞

from dapper.fsharp.

Any change is possible and since we are following SemVer, I am ready to scratch whatever is necessary to make it better. 😄

from dapper.fsharp.

Good idea.
I'll take a crack at trying to implement support for multiple columns on the Linq join and leftJoin very soon. I think that's the only feature that is missing for parity with the base API. Hopefully the join syntax will allow it.. if not, something a creative workaround may be necessary. 😬

from dapper.fsharp.

No problem at all! It was a quick one and you did all the hard work. 🙏

Exactly as you wrote, when having the multicolumn joins it will be quick. I also plan to add link to the "pre-v3" docs to have people using older version less confused. 😀

from dapper.fsharp.

The multi-column join went really well! 🎉

    testTask "Inner Join Multi-Column" {
        let query = 
            select {
                for l in table<MultiJoinLeft> do
                innerJoin r in table<MultiJoinRight> on ((l.Key1, l.Key2) = (r.Key1, r.Key2)) 
                selectAll
            }

        Expect.equal query.Joins [
            InnerJoinOnMany ("MultiJoinRight", ["Key1", "MultiJoinLeft.Key1"; "Key2", "MultiJoinLeft.Key2"])
        ] ""
    }

    testTask "Left Join Multi-Column" {
        let query = 
            select {
                for l in table<MultiJoinLeft> do
                leftJoin r in table<MultiJoinRight> on ((l.Key1, l.Key2) = (r.Key1, r.Key2)) 
                selectAll
            }

        Expect.equal query.Joins [
            LeftJoinOnMany ("MultiJoinRight", ["Key1", "MultiJoinLeft.Key1"; "Key2", "MultiJoinLeft.Key2"])
        ] ""
    }

I had to temporarily butcher the test project by removing all the broken stuff, but I saw that you are working on the tests next, so I think I will just wait for you to complete the test rework, and then I'll get a fresh pull of the fixed branch to apply the multi-join feature. That should keep things simple.

BTW, one workaround I implemented that you might consider to get the tests up and running quickly was to add a Legacy.fs file to the Tests project with the old where filter functions (since they were used all over the place to unit test the Linq queries):

/// Added temporarily to fix tests
module Dapper.FSharp.Legacy

/// Creates WHERE condition for column
let column name whereComp = Where.Column(name, whereComp)
/// WHERE column value equals to
let eq name (o:obj) = column name (Eq o)
/// WHERE column value not equals to
let ne name (o:obj) = column name (Ne o)
/// WHERE column value greater than
let gt name (o:obj) = column name (Gt o)
/// WHERE column value lower than
let lt name (o:obj) = column name (Lt o)
/// WHERE column value greater/equals than
let ge name (o:obj) = column name (Ge o)
/// WHERE column value lower/equals than
let le name (o:obj) = column name (Le o)
/// WHERE column like value
let like name (str:string) = column name (Like str)
/// WHERE column not like value
let notLike name (str:string) = column name (NotLike str)
/// WHERE column is IN values
let isIn name (os:obj list) = column name (In os)
/// WHERE column is NOT IN values
let isNotIn name (os:obj list) = column name (NotIn os)
/// WHERE column IS NULL
let isNullValue name = column name IsNull
/// WHERE column IS NOT NULL
let isNotNullValue name = column name IsNotNull

from dapper.fsharp.

Maaaan, you are making it harder and harder for me. 😄 I totally understand and love the compile-time check for LINQ builders, but I am afraid the "old version" is too distant from the LINQ one.

Maybe... and this is a heretic idea... we should make v3.0 dropping old version completely and stay with LINQ only? 🤔

from dapper.fsharp.

This calls for a for a pros & cons list 😁:

Removing Base API

Pros

• Easier to maintain one API surface vs two
• Eliminates concerns over maintaining parity between the two APIs
• Simplifies the readme page to list only one API

Counter arguments

• Is it that much harder to maintain both APIs?
• Does it matter if the base and Linq APIs are not the same?
• Base API could be preserved but moved off of the main readme to a wiki page to minimize it and to ensure that new users gravitate toward the Linq API

Cons

• There are inevitably some people still using the base API. (I know this for a fact because the last PR was to allow multiple join conditions, and that change was only made to the base API)

Counter arguments

• People always have the option to stay on an older version if they really need the base API
• The multi-join condition logic could be merged into the Linq API

from dapper.fsharp.

Thanks for that list! I was always afraid of big changes, but only in the scope of the same major version. If I take it to an extreme, v3 can be totally different from v2 and nobody should say anything, because hell that's what's SemVer for, right? Of course, we should keep things as continuous as possible.

So the main concern for me is: can LINQ API fully replace the old one? In other words: Is there any dark corner of functionality where the old API can do something which the new cannot? 🤔 I don't think so, but you as the author of LINQ one should know better...

from dapper.fsharp.

The Linq api should be able to handle all of the current base api functionality.
So I think it would just be a matter of porting over any missing features, which i don’t think there are many. The new multiple join conditions is the only thing I can think of at the moment. That and replacing the anonymous update set.

from dapper.fsharp.

Yup, once we step in having LINQ only, then the anonymous update is no more.

To explain why I tend to remove old API: I was pairing with one of the customers and I saw how people love type-safety. Once you have an option to use LINQ API, why would you use the old one? I was a great start and I am happy for that, but now, thanks to you and your code, we can just drop the old one as "outdated".

from dapper.fsharp.

Good timing on the new bug #41 because I think that this feature could be implemented differently in the Linq API:
Instead of having an overload that takes multiple column equality joins, I think it could/should only require a single custom operation that works similarly to the where operation -- meaning that instead of supporting a collection of column equality joins, it should be able to support a fuller expression similar to the where implementation:

type Where =
    | Empty
    | Column of string * ColumnComparison
    | Binary of Where * BinaryOperation * Where
    | Unary of UnaryOperation * Where
    | Expr of string

So you could this:

select {
    for p in personTable do
    join d in dogsTable on (p.Id = d.OwnerId && d.Nickname = "Sparky")
    orderBy p.Position
} 
|> conn.SelectAsync<Person, Dog>

This would require some changes in Dapper.FSharp.fs to support. (Although I'm not 100% if this is possible yet -- just an idea).

#41 also hints that many people are still using the base API.

from dapper.fsharp.

For the next version, I am more and more convinced to do the bold move and ...

• drop the old API entirely
• targetting at least .NET 5 (having .NET 6 LTS pretty soon)
• simplify the docs

from dapper.fsharp.

I will setup a separate version3 branch soon so you can see (and contribute 🤞😄) where is it going.

from dapper.fsharp.

I also found that the old API has leftJoin and innerJoin while the new has join and leftJoin... maybe we should look at the keywords names while messing with API for v3 😄

from dapper.fsharp.

I'm fine with changing it to innerJoin, but the reason I called it join was to match the core F# query computation expression, which is also used by SQLProvider, and SqlHydra.
So once the base API goes away, this would be the only API that uses innerJoin.

With that said, if you have a strong preference for innerJoin to match leftJoin, we can totally do that!

from dapper.fsharp.

Really appreciate your commitment, Sir! I had a madness week and the next one doesn't look better, but I'll try to work on v3 hopefully. 🤞

from dapper.fsharp.

No worries at all. Do you want to create a new v3 branch first or should i just start on a temp branch?

from dapper.fsharp.

Hi friend, I just pushed the latest changes to version3 (old implementation is removed) - now need to refactor tests 😁

from dapper.fsharp.

Sorry, next time I'll stick to the more traditional patch workflow. This obviously has caused unnecessary work for you to keep things organized when you've already said you are having a busy week! 😅

Just to recap version3 changes:

• Add multi-column joins
• Remove base API
• Update docs

Is that it? If so, that should be able to happen fairly quickly.

from dapper.fsharp.

Tests are fixed now in version3 branch! 🥳

from dapper.fsharp.

Also with docker-compose for better testing ;) I'll plug it later into FAKE target to compose-up, test, componse-down 😄

from dapper.fsharp.

Also with docker-compose for better testing ;) I'll plug it later into FAKE target to compose-up, test, componse-down 😄

Ooo nice!

from dapper.fsharp.

And now with MSSQL dockerized as well. Now it should be dead simple to run all necessary tests.

I assume it's yours now @JordanMarr for multicolumn stuff and I'll update README later and let's release a v3.0.0

from dapper.fsharp.

I will add the multi column join support and also rework the update to use the more strongly typed ‘set’ syntax.

from dapper.fsharp.

Thanks a lot, Sir!

from dapper.fsharp.