mattwar/iqtoolkit

Exception invalid argument types

Closed this issue · 6 comments

Hi Matt,

thanks for the last fix but i have another problem. I created a test case for that issue:

Nullable<Boolean> hasOrders = null;

var query = db.Customers.Select(r => new
{
    CustomerID = r.CustomerID,
    HasOrders = hasOrders != null
                      ? (bool)hasOrders
                      : db.Orders.Any(o => o.CustomerID.Equals(r.CustomerID))
});

var test = query.ToList();

At some point in the PartialEvaluator the first case of the mini-if is evaluated and this leads to an invalid cast because hasOrders is null.

Please ignore the proper sense of the query it is only a simplified test case :-)

Thanks and best regards,

Markus

You want the cast to be to (bool?) instead of (bool). The PartialEvaluator is isolating the expressions that are not dependent on the database and evaluating them up front. Your cast to bool is what is failing.

As i said the code snippet is simplified and is maybe a little bit unlogical but i want a (bool) in my result (HasOrders).

The snippet should demonstrate the following:
I have the situation that not every case could be evaluated. In the above example the first case can not be evaluated because hasOrders is null. But it is not necessary to evaluate it because the second case is valid. If hasOrders is not null the first case is valid and the cast in that case is valid.

In my fix i rewrite the conditional expression to the valid case (in my example the second one) and eliminating the invalid first case. If the condition could be evaluated upfront we can rewrite the whole expression.

I hope you understand me :-) It's a little bit hard to describe.

Because the second half of the conditional (?:) operator is dependent on the database, then the whole expression is, and becomes part of the database query, except for the isolated pieces, like the '(bool)hasOrders' expression. This is evaluated locally and passed as a parameter to the query. That means that the C# code '(bool)hasOrders' is always executed, because the parameters are always computed first before the query is executed. This separation of the query into local and database portions is taking precedence over any language rules like the short-circuiting C# conditional operator (which gets translated to a SQL case expression typically.)

You could probably use 'hasOrders ?? false' instead of '(bool)hasOrders'.

I've been considering this more and I think it makes sense to make this change. However, it would need to also apply to other short circuit operators.

I've been working on a larger refactoring and move to dotnet standard (its the 'standard' branch). I'd rather make this kind of change there instead. I'll add it to my todo list.

Thanks Matt

This change has been merge in to the master branch.