zzzprojects / Eval-Expression.NET

C# Eval Expression | Evaluate, Compile, and Execute C# code and expression at runtime.
https://eval-expression.net/
Other
459 stars 86 forks source link

Querying dynamic data with generated LINQ queries #47

Closed TehWardy closed 5 years ago

TehWardy commented 6 years ago

Hey @JonathanMagnan,

Here's some more detail on an issue i'm having which relates closely to #44 but I didn't feel right in canabalizing @tomasceleda's issue for my own problem (feel free to close and merge if you think these are the same problem though).

I put together a fiddle (seems to be the best way to hand over a complete "working" sample of the problem ...

https://dotnetfiddle.net/ZZ4V1B

... In the sample you can see that I can "manually execute the query" but I can't dispatch the same code to Eval.Execute() for some reason.

testResult = ... on line 73 of the code sample runs fine. Given the same code to Eval.Execute() as a string on line 102 it fails with a strange exception I don't understand how to parse.

JonathanMagnan commented 6 years ago

Hello @TehWardy ,

It looks we really missed the follow-up on your request.

I was about to answer you on the other thread by asking for a Fiddle since our example is working: https://dotnetfiddle.net/tzeCbZ

But it looks you already provided one.

We will start to look at it tomorrow.

Sorry for the delay.

Best Regards,

Jonathan

JonathanMagnan commented 6 years ago

Hello @TehWardy ,

We have spent quite a few times in the past day on that issue but dynamic always cause some headache and we are not quite sure that we will succeed to support it or not in that scenario.

However, we have made some progress, so we are still looking at it.

I will try to keep you updated but we cannot make any promise on this request due to the complexity.

Best Regards,

Jonathan

TehWardy commented 6 years ago

Hi @JonathanMagnan,

thanks for keeping me up to date.

Interestingly enough ... i have the same problem in our front end code that will be providing the metadata to the code that generates the code i feed in to your library.

I think the issue is in .Nets handling of the generic when used like this ...

Foo<dynamic>

... When the receiver is given that if you use reflection to get the type information you actually get back the same as if you did ...

var type = typeof(object);

... With ExpandoObjects however you can basically say that typeof(ExpandoObject) will automatically be mapped to dynamic variables to force the compiler to treat the object as dynamic.

For example ...

var x = new Message<object>();
var genericType = x.GetType().GetGenericArguments()[0];

if (genericType == typeof(ExpandoObject) || genericType == typeof(object))
{
    dynamic y = x;
}

... from that point on the variable y is a dynamic version of x and the compiler will allow you to do dynamic things with it but consuming x in the same way is not allowed.

I also found from the stack overflow issue that Jon Skeet pointed out when you have a dynamic variable that has a non dynamic property, in the compiler you're still working within a dynamic sub context weather you like it or not, the fact is there are no "compile time checks" that the compiler can actually do past the point you start consuming any var declared as "dynamic something =".

For my code to work, i took on this assumption whenever I saw something that was of type object or ExpandoObject and the net result was that my code worked but it meant that I had to take responsibility for making sure my dynamic code was correct before making the call.

It's also why i had to the JSON dance on line 119 of my example on the fiddle. since json is just a string and how it's interpretted is down to how I call the json converter it's my call to turn the dynamic result in to something concrete where I know that the conversion can be made, but only when that's true. In the scenario where it isn't i return a dynamic result which follow on code interprets as "typeof(object)" weather i like it or not.

I hope this helps, shout if there's anything else I can offer you I learned a ton by digging through the roslyn code and by picking Jon Skeets brains over the past 4 or 5 years on this exactly problem as pretty much everything we do ends up in this kind of problem.

Funnily enough ... my dependency on Eval here was me thinking "lets make this issue permanently someone elses problem" ... I should have known better ;)

JonathanMagnan commented 5 years ago

Hello @TehWardy ,

Great news, we have made a lot of progress on that issue.

Your code currently works but some cleanup is still required for our fix.

We were so much focusing to find why the dynamic keyword + LINQ was not working that we missed the real issue.

The first issue is caused by how our library solves generic type. (IEnumerable<dynamic>) was returning IEnumerable since a type with IEnumerable exists and the cast was not forcing to solve all types. Obviously, following LINQ cannot work correctly on this interface.

It looks our library was already working very fine with dynamic keyword + LINQ.

There are still a few changes to makes but my guess is next Monday, a new version will be released that will support this scenario and a lot of more involved dynamic keyword.

Best Regards,

Jonathan

TehWardy commented 5 years ago

oh that's awesome news @JonathanMagnan thanks for the update :) Can't wait to see this in my solution on monday !!!

JonathanMagnan commented 5 years ago

Hello @TehWardy ,

The v2.8.6 has been released yesterday.

Could you try it and let me know if everything works as expected? (It currently doesn't work with.NET Fiddle since anonymous type creation is not supported).

I'm not sure if we have gone too far or not in this request by automatically supporting dynamic type even if some case could have been object type. So we might need to rollback some changes if we introduced some issue but it will be always possible to achieve the same behavior by enabling the ForceObjectAsDynamic option.

There is A LOT of new thing that we have added to better support dynamic type, so let me know if something else is missing.

Best Regards,

Jonathan

TehWardy commented 5 years ago

Hi @JonathanMagnan, I have re-run the sample i uploaded to fiddle and it died, then i noticed you mentioned that the fiddle wasn't working (presumably because of the reason you outlined) so I ran the local copy of that that I have and that gave me a cast exception.

I would send you the zipped up project but it's just as easy for you to copy paste the fiddle code in to a new console app (just add nuget refs for Newtonsoft.Json and ZZZ.Expressions.Eval and you're sorted), that and github to my knowledge won't let me attach a zip to a bug thread.

JonathanMagnan commented 5 years ago

You are right,

I thought we had the same code as you but it look I removed this line by mistake which throw this cast error:

FaceValue = group.Sum(i => (decimal)i.FaceValue)

We will look at it.

TehWardy commented 5 years ago

Yeh I figure you would know more than me all i got was this ...

   at lambda_method(Closure , <>f__AnonymousType0`6 )
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Linq.Enumerable.Sum(IEnumerable`1 source)
   at lambda_method(Closure , IGrouping`2 )
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at lambda_method(Closure , Object )
   at Z.Expressions.EvalContext.Execute[TResult](String code, Object parameters)
   at Z.Expressions.Eval.Execute[TResult](String code, Object parameters)
   at System.Dynamic.UpdateDelegates.UpdateAndExecute3[T0,T1,T2,TRet](CallSite site, T0 arg0, T1 arg1, T2 arg2)
   at Program.Main() in C:\Users\me\Desktop\ZZZ\ZZZ\Program.cs:line 102
JonathanMagnan commented 5 years ago

The v2.8.7 has been released.

I double checked with the original code to make sure we didn't forgot anything this time ;)

The issue was caused because on our side, the code is similar to:

object a = 1.1;
decimal b = (decimal)a;

which throw a Cast error. In this situation, we will now assume the variable a is of type dynamic instead.

Let me know if this new version works as expected.

Best Regards,

Jonathan

TehWardy commented 5 years ago

@JonathanMagnan we've managed to get this working now. I'll keep testing around this area though and let you know if anything else comes up.

Thanks for your work on this!