Improve compiler support in Linq partial updates

[JordanMarr]

The Linq update builder currently accepts an anonymous record for partially updating a table. I think it is worth considering deprecating the anonymous record support (on the Linq builder only) in favor of a setColumn (or similarly named) operator that allows you to set individual columns.

Why? Better compiler support for partial updates with less chance of run-time exceptions!

• setColumn can be constrained to 'T which will allow the F# compiler to provide type check errors at compile-time, whereas the anonymous record approach allows potential run-time errors.

For example:

update {
    for p in personTable do
    set {| FirstName = "UPDATED"; LastName = "UPDATED" |}
    where (p.Position = 1)
} 
|> conn.UpdateAsync

would become:

update {
    for p in personTable do
    setColumn p.FirstName "UPDATED" // constrained to only allow properties on `p`
    setColumn p.LastName "UPDATED"
    where (p.Position = 1)
} 
|> conn.UpdateAsync

• A less important reason (but still a nice side effect) is that removing support for anonymous records it will greatly simplify the Linq update builder implementation by removing the extra 'U generic parameter.

[Dzoukr]

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? 🤔

[JordanMarr]

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

[Dzoukr]

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...

[JordanMarr]

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.

[Dzoukr]

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".

[JordanMarr]

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.

[Dzoukr]

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

[Dzoukr]

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

[Dzoukr]

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

[JordanMarr]

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. 😬

[Dzoukr]

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 😄

[JordanMarr]

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!

[JordanMarr]

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

[Dzoukr]

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. 🤞

[JordanMarr]

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

[Dzoukr]

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

[JordanMarr]

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.

[Dzoukr]

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. 😀

[JordanMarr]

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

[Dzoukr]

Tests are fixed now in version3 branch! 🥳

[Dzoukr]

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

[JordanMarr]

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

Ooo nice!

[Dzoukr]

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

[JordanMarr]

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

[Dzoukr]

Thanks a lot, Sir!