Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ascott18 opened this issue Oct 26, 2017 · 3 comments
Closed

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

ascott18 opened this issue Oct 26, 2017 · 3 comments
Assignees
Labels

Comments

@ascott18
Copy link
Contributor

I ran into a real head scratcher today, and I think I figured out the root cause. I wrote up the whole issue at IntelliTect/Coalesce#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 StefH self-assigned this Oct 27, 2017
@StefH StefH added the bug label Oct 27, 2017
@StefH
Copy link
Collaborator

StefH commented Oct 27, 2017

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 StefH closed this as completed in 83e245d Oct 27, 2017
@StefH
Copy link
Collaborator

StefH commented Oct 27, 2017

New NuGet added.

@ascott18
Copy link
Contributor Author

Thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants