-
Notifications
You must be signed in to change notification settings - Fork 35
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
Improve compiler support in Linq partial updates #40
Comments
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? 🤔 |
This calls for a for a pros & cons list 😁: Removing Base APIPros
Counter arguments
Cons
Counter arguments
|
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... |
The Linq api should be able to handle all of the current base api functionality. |
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". |
Good timing on the new bug #41 because I think that this feature could be implemented differently in the Linq API: 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. |
Any change is possible and since we are following SemVer, I am ready to scratch whatever is necessary to make it better. 😄 |
For the next version, I am more and more convinced to do the bold move and ...
|
I will setup a separate |
Good idea. |
I also found that the old API has |
I'm fine with changing it to With that said, if you have a strong preference for |
I think I just imagined a way to implement multi column joins in the Linq API that might work... 🤞 |
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. 🤞 |
No worries at all. Do you want to create a new v3 branch first or should i just start on a temp branch? |
Hi friend, I just pushed the latest changes to |
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:
Is that it? If so, that should be able to happen fairly quickly. |
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. 😀 |
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 /// 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 |
Tests are fixed now in |
Also with |
Ooo nice! |
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 |
I will add the multi column join support and also rework the update to use the more strongly typed ‘set’ syntax. |
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:
would become:
'U
generic parameter.The text was updated successfully, but these errors were encountered: