Are multi column joins no longer supported?
travis-leith opened this issue · 9 comments
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
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 One or more errors occurred. (The given key was not present in the dictionary.)
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 docker compose up -d
and run all the tests easily.
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 DateTimeOffset.MaxValue
. If I replace this with a join on a string field in the tuple, it works just fine. Any thoughts?
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# join
on
syntax adds constraints that would make it impossible to express certain kinds of join filter clauses.
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 AND
or OR
in the on
statement, this would be impossible to express because the first clause is =
, but the second clause is >
.
For that reason, I would suggest getting into the habit of moving these filters to the where
clause:
return! select {
for o in leftTable do
leftJoin v in rightTable on (o.Id = v.OrderId)
where (v.ValidTo = DateTimeOffset.MaxValue)
selectAll
}
Incidentally, the where
handler is much more robust and would already accommodate the DateTimeOffset.MaxValue
without having to capture it in a binding ahead of time.
In hindsight, we probably should have just made it a requirement to move join
filtering to the where
clause rather than adding partial support for doing it in the join
statement (as was done due to a previous user request). Because it simply can't handle the full range of expressions required if going down that path.
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. 💪