Dzoukr / Dapper.FSharp

Lightweight F# extension for StackOverflow Dapper with support for MSSQL, MySQL, PostgreSQL, and SQLite
MIT License
365 stars 35 forks source link

Are multi column joins no longer supported? #77

Open travis-leith opened 1 year ago

travis-leith commented 1 year ago

This feature was discussed and implemented: https://github.com/Dzoukr/Dapper.FSharp/issues/62 and tests were written: https://github.com/Dzoukr/Dapper.FSharp/pull/63/files#diff-7a8dde6aa28f58c34432379d8cfa27c1b7ec21029d248424056110332d7cd49f But I no longer see any similar tests

travis-leith commented 1 year ago

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?

Dzoukr commented 1 year ago

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?

travis-leith commented 1 year ago

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.

Dzoukr commented 1 year ago

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

travis-leith commented 1 year ago

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.

travis-leith commented 1 year ago

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?

travis-leith commented 1 year ago

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.

travis-leith commented 1 year ago

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

JordanMarr commented 1 year ago

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