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

Possible feature: Add support for conditional joins over ConstantExpression #62

Closed Dzoukr closed 2 years ago

Dzoukr commented 2 years ago

Currently, there is no way how to do conditional join.

I can use trick with multi-column (tuples):

let query = 
    select {
        for l in table<MultiJoinLeft> do
        leftJoin r in table<MultiJoinRight> on ((123, l.Key2) = (r.Key1, r.Key2)) 
        selectAll
    }

...but this fails with NotImplementedException, because of ConstantExpression.

Need to investigate how difficult this could be to add...

cc: @JordanMarr : Any thoughts on this? ๐Ÿค”

Dzoukr commented 2 years ago

Btw the best syntax would be

select {
    for l in table<MultiJoinLeft> do
    leftJoin r in table<MultiJoinRight> on ((l.Key1 = 123) && (l.Key2 = r.Key2))
    selectAll
}

But need to investigate, how to proceed...

JordanMarr commented 2 years ago

Oh I think that should actually a really easy fix. Iโ€™ll take a stab at removing the error message so you can manually enter a value.

Dzoukr commented 2 years ago

I am afraid it won't be that easy...

type Join =
    | InnerJoin of table:string * colName:string * equalsToColumn:string
    | LeftJoin of table:string * colName:string * equalsToColumn:string
    | InnerJoinOnMany of table:string * List<string * string>
    | LeftJoinOnMany of table:string * List<string * string>
JordanMarr commented 2 years ago

I think the tuple on syntax would map to LeftJoinOnMany which would probably be the easiest approach that would avoid having to change the InnerJoin and LeftJoin DUs.

But if I am understanding you correctly, implementing the "ideal syntax" example would involve changing to something like this:

type Join =
    | InnerJoin of table:string * onClause:Where
    | LeftJoin of table:string * onClause:Where
    | InnerJoinOnMany of table:string * List<string * string>
    | LeftJoinOnMany of table:string * List<string * string>

Which seems like that might be a good way to go. In fact that might even eliminate the need for InnerJoinOnMany and LeftJoinOnMany, which could actually be a nice consolidation:

type Join =
    | InnerJoin of table:string * onClause:Where
    | LeftJoin of table:string * onClause:Where
Dzoukr commented 2 years ago

I absolutely love it! That's clean as hell!

JordanMarr commented 2 years ago

It looks like the on query syntax may not allow it. ๐Ÿ™ˆ

image

If that's the case, then the only path forward may be to continue using tuples and fix that constant bug.

JordanMarr commented 2 years ago

Well so much for clean and awesome syntax... It seems that the joined table (on the right side of the =) must be an actual column because that provides a MemberInfo that provides the DeclaringType property which is used to lookup the joined table name. Which means that the constant needs to be on the left side. But as I look to your original post, I see that you were already doing that anyway, so maybe not a big deal.

These two tests now pass: image

But that's only because I am passing in numeric values instead of a column name. The problem being that InnerJoinOnMany and LeftJoinOnMany still take strings for column names.

Dzoukr commented 2 years ago

Ok man, thanks a lot for digging into it... I think we would have to wrap MemberInfo into some DU case ๐Ÿค”

I am not definitely giving up on this! ๐Ÿ˜€

JordanMarr commented 2 years ago

Thatโ€™s exactly what I did for the builder/expressions part. I will start a draft PR with what I have done so far.

Dzoukr commented 2 years ago

Fixed in v3.3