-
Notifications
You must be signed in to change notification settings - Fork 36
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
Are multi column joins no longer supported? #77
Comments
I changed to version 3.4 but I am still not able to get this to work. Here is my syntax select {
for o in leftTable do
//leftJoin v in rightTable on (o.Id = v.OrderId)
leftJoin v in rightTable on ((o.Id, DateTimeOffset.MaxValue) = (v.OrderId, v.ValidTo))
selectAll
} working against sql server. The error I am getting is The commented out left join works fine btw. Am I missing something? |
Can you try the latest version, please? It should be still supported 🤔 Maybe, if it doesn't work, can you write a simple test and make PR, I will try to have a look at it? |
I have forked/cloned but I cannot run the tests because the only Sql server database I have access to is Azure SQL Database, which does not support switching databases in a connection. See here (https://social.msdn.microsoft.com/Forums/en-US/1d83c593-095d-416b-b927-7a6489503131/sql-quotusequot-statement-is-not-supported-in-azure-sql-database-for-selecting-different?forum=ssdsgetstarted) So the tests are not compatible with Azure Sql DB. I can try to adjust all the MsSql tests so that they use a dedicated init connection for set up, then use a different connection to run the tests. Not sure if this would be acceptable to you. |
It's all based on Docker. 😃 You can run See: https://github.com/Dzoukr/Dapper.FSharp/blob/master/docker-compose.yml |
I will have to wait until I get home to get access to a machine with docker, but in the meanwhile I have adjusted the test setup to work for Azure Sql DB and added the following test [<Test>]
member _.``select with multi column left join``() =
task{
do! init.InitPersons()
do! init.InitDogs()
let persons = Persons.View.generate 10
let dogs = Dogs.View.generate1toN 5 persons.Head
let! _ =
insert {
into personsView
values persons
} |> conn.InsertAsync
let! _ =
insert {
into dogsView
values dogs
} |> conn.InsertAsync
let! fromDb =
select {
for p in personsView do
leftJoin d in dogsView on ((p.Id, "Dog 1") = (d.OwnerId, d.Nickname))
selectAll
} |> conn.SelectAsyncOption<Persons.View, Dogs.View>
Assert.AreNotEqual(fromDb |> Seq.length, 0)
} which passes just fine. So I have no idea what is wrong with my other code. |
aha! The problem is |
The following seems to work let maxDate = DateTimeOffset.MaxValue
return! select {
for o in leftTable do
//leftJoin v in rightTable on (o.Id = v.OrderId)
leftJoin v in rightTable on ((o.Id, maxDate) = (v.OrderId, v.ValidTo))
selectAll
} not sure if you would consider this a bug or not. If not, might be worth documenting somehow. |
@Dzoukr I will leave it up to you to decide to close this issue or leave it open until the "bug" is either fixed or documented. |
This would be an easy fix. However, the F# For example, what if you need to do this: SELECT *
FROM leftTable o
LEFT JOIN rightTable v ON (o.Id = v.OrderId AND v.ValidTo > @maxDate) Since F# doesn't support multiple clauses separated by For that reason, I would suggest getting into the habit of moving these filters to the return! select {
for o in leftTable do
leftJoin v in rightTable on (o.Id = v.OrderId)
where (v.ValidTo = DateTimeOffset.MaxValue)
selectAll
} Incidentally, the In hindsight, we probably should have just made it a requirement to move In fact, it would probably be a good idea to steer users down that path in the documentation and downplaying tuple join clauses in general (even though they are partially supported), since at some point a user will have a more complex join filter that can only be handled that way. But I'm glad you were able to find a solution. 💪 |
This feature was discussed and implemented: #62
and tests were written: https://github.com/Dzoukr/Dapper.FSharp/pull/63/files#diff-7a8dde6aa28f58c34432379d8cfa27c1b7ec21029d248424056110332d7cd49f
But I no longer see any similar tests
The text was updated successfully, but these errors were encountered: