zzzprojects
Unable to infer types for lambda arguments
This issue is potentialy related or equal to:
https://github.com/StefH/System.Linq.Dynamic.Core/issues/288 https://github.com/StefH/System.Linq.Dynamic.Core/issues/259
I prepared a simple test case:
var users = new[]
{
new { name = "Juan", age = 25 },
new { name = "Juan", age = 25 },
new { name = "David", age = 12 },
new { name = "Juan", age = 25 },
new { name = "Juan", age = 4 },
new { name = "Pedro", age = 2 },
new { name = "Juan", age = 25 }
}.ToList();
IQueryable query;
string results;
// This does work
query = users.AsQueryable();
query = query.GroupBy(CustomParsingConfig.ParsingConfig, "new(name as name)", "it");
query = query.Select("new (it.Key as Key, new(it.Sum(x => x.age) as ageSum) as nativeAggregates, it as Grouping)");
results = JsonConvert.SerializeObject(query);
// This miserable fails :)
query = users.AsQueryable();
query = query.GroupBy(CustomParsingConfig.ParsingConfig, "new(name as name)", "it");
query = query.Select("new (it.Key as Key, new(Enumerable.Sum(it, x => x.age) as ageSum) as nativeAggregates, it as Grouping)");
results = JsonConvert.SerializeObject(query);
I've been looking through the code and the fix looks quite complex.
What happens now is that the ParseArguments() method in ExpressionParser does not properly handle generic types or I even presume specific types for lambda expressions.
Expression[] ParseArguments()
{
var argList = new List<Expression>();
while (true)
{
var argumentExpression = ParseConditionalOperator();
_expressionHelper.WrapConstantExpression(ref argumentExpression);
argList.Add(argumentExpression);
if (_textParser.CurrentToken.Id != TokenId.Comma)
{
break;
}
_textParser.NextToken();
}
return argList.ToArray();
}
This code is basically parsing the arguments one by one, without any previous knowledge of any potential relationship between them, or where are they consumed.
For example, take this method signature:
public string DoRandomStuff<T>(Func<T, string> projection, T arg) {
return projection(arg) + ";";
}
If we want to call this using DynamicLinq:
var users = new[]
{
new { name = "Juan", age = 25 },
new { name = "Juan", age = 25 },
new { name = "David", age = 12 },
new { name = "Juan", age = 25 },
new { name = "Juan", age = 4 },
new { name = "Pedro", age = 2 },
new { name = "Juan", age = 25 }
}.ToList().AsQueryable();
var test = users.Select("DoRandomStuff(~, x => x.age)");
If this previous code works, it's just a coincidence, because when parsing the arguments for DoRandomStuff the ParseArguments() and related logic is not saying, ok so parameter 1 for the method is of type T, then X must be of type T.
Solving the issue is difficult because you don't have precise knowledge of what overload (if the method has many overloads) you need to use to do this type inference. Indeed, this cannot be exactly known until you parse all arguments which is fish that bite's its tail problem.
A different approach would be to parse all lambdas assuming lambda arguments are dynamic types, and once all method arguments have been parsed, have some logic resolve what is the most suitable overload of the method to use, and then replace (or rebuild the expression) with the properly inferred types.
And this looks like the proof in the documentation that custom handling was added for some specific cases, but there is no proper generic support for lambdas as method arguments:
I am still anaylizing how lambdas are parsed, and it also looks like it wont support multiple argument lambdas:
(p, q) => p.a + q.b
It seems to fail parsing with an object not set to the instance of an object error.
@StefH I mode some good progress in a POC to remove ad-hoc logic for aggregates in favor of generic handling of lambdas.
#295
Tests are not 100% passing, but before continuing work on this I'd like a second opinion on the strategy followed.
¿Also, how aceptable would it be to have a new major release that breaks backwards compatiblity?
My problem now is that, after proper implementation of lambda/method call parsing, code like this:
var qry = p.GroupBy("Item1", "it").Select("it.Max(it.Item3)");
Is not valid syntax, it should be:
var qry = p.GroupBy("Item1", "it").Select("it.Max(p => p.Item3)");
But these changes open the door to being able to expand the complexity of calls to method supported by this library, including nested calls and multi-dimensional lambda's.
Would it be an option to make this configurable? So the default behavior is the new logic. And using a config-setting, you can fallback to old logic?
btw I've added some comments to you PR, most of them are just questions.
Would it be an option to make this configurable?
Yes I was thinking on working in a global cleanup, then adding a backwards compatiblity layer that is configurable by the user.
The PR needs much more work, will keep updating.
Thanks!