zzzprojects / System.Linq.Dynamic.Core

The .NET Standard / .NET Core version from the System Linq Dynamic functionality.
https://dynamic-linq.net/
Apache License 2.0
1.56k stars 228 forks source link

Type conversions generated in cases where they're not needed. #107

Closed ascott18 closed 6 years ago

ascott18 commented 6 years ago

I ran into a real head scratcher today, and I think I figured out the root cause. I wrote up the whole issue at https://github.com/IntelliTect/Coalesce/issues/25, which I'd encourage a read through of to get context for this. Look specifically at the (System.Object) cast in the second query's DebugView.

Basically, it seems that type conversions are being generated in cases where they aren't strictly needed, and this is causing EF Core to get confused when interpreting query results. The fault partly lies with EF Core, but I figure a change here will be easier to accomplish than a change to EF Core.

I believe the culprit to be this spot: https://github.com/StefH/System.Linq.Dynamic.Core/blob/master/src/System.Linq.Dynamic.Core/ExpressionParser.cs#L1464

When there is a literal null as one of the two operands in an equality operator, I'm guessing that it is assumed to have type "System.Object", and so the left-hand side is being explicitly converted to System.Object. Would it be possible to add handling here to check for literal null and skip the emission of a conversion in such cases? Or am I way off base here on what is causing this cast to be created?

Thanks for taking a look, and thanks for maintaining this library!

StefH commented 6 years ago

It's indeed related to the NullLiteral, but the issue is found here:

https://github.com/StefH/System.Linq.Dynamic.Core/blob/master/src/System.Linq.Dynamic.Core/ExpressionParser.cs#L654

I need to add an extra check here for NullLiteral.

StefH commented 6 years ago

New NuGet added.

ascott18 commented 6 years ago

Thank you so much!