-
-
Notifications
You must be signed in to change notification settings - Fork 518
Description
Hello!
I've encountered a bug (at least, what I interpret to be one) in the Query.Join
method:
public partial class Query
{
// ...
public Query Join(string table, Func<Join, Join> callback, string type = "inner join")
{
return Join(j => j.JoinWith(table).Where(callback).AsType(type));
}
public Query Join(Query query, Func<Join, Join> onCallback, string type = "inner join")
{
return Join(j => j.JoinWith(query).Where(onCallback).AsType(type));
}
}
I wrote something like this:
var query = new Query("orders")
.Join("shipments", j => j.On("orders.id", "shipments.id").AsLeft());
The issue is fairly clear in reviewing the SqlKata source code, but since Join
has these methods:
public class Join : BaseQuery<Join>
{
// ...
public Join AsInner() => AsType("inner join");
public Join AsOuter() => AsType("outer join");
public Join AsLeft() => AsType("left join");
public Join AsRight() => AsType("right join");
public Join AsCross() => AsType("cross join");
}
...I had assumed I could use them in this context.
However, since Query.Join
has the optional parameter string type = "inner join"
, the query that results from my example is overridden with AsType("inner join")
.
It's easily resolved by either calling either of these:
// don't use any As...() methods, and instead populate the optional parameter
query.Join("shipments", j => j.On("orders.id", "shipments.id"), "left join");
// use Query.LeftJoin
query.LeftJoin("shipments", j => j.On("orders.id", "shipments.id"));
But it wasn't obvious based on the available methods that this was expected.
I'm not sure what the appropriate fix is. Seeing now that callback
is passed into Where
, it seems like callers are expected only to be setting join conditions in that callback. Fair enough, but then I'm not sure when calling AsLeft
would be relevant, if not in a join callback.
My immediate reaction would be to add overloads like this (just presenting string
since the methods are otherwise the same):
public Query Join(string table, Func<Join, Join> callback)
{
return Join(j => j.JoinWith(table).Where(callback));
}
public Query Join(string table, Func<Join, Join> callback, string type)
{
return Join(j => j.JoinWith(table).Where(callback).AsType(type));
}
I believe that should work since "inner join"
is the default value for the type anyway, so this would allow the value to be changed in the callback. That still allows for the nonsensical query.LeftJoin("shipments", j => j.On("orders.id", "shipments.id").AsLeft(), "inner join")
, but I think that would be pretty clearly erroneous.