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.

Sign up for GitHub

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

Jump to bottom

Query : ToString() method that takes a formatter argument should do client evaluation #7364

Closed
maumar opened this issue Jan 5, 2017 · 13 comments
Closed

Query : ToString() method that takes a formatter argument should do client evaluation #7364

maumar opened this issue Jan 5, 2017 · 13 comments
Assignees
@smitpatel
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone
2.0.0

Comments

@maumar
Copy link
Contributor

maumar commented Jan 5, 2017

Currently ToString calls with formatter (e.g ToString("X")) are translated to SQL and the formatter is ignored, we should instead translate it on the client.
maumar added a commit that referenced this issue Jan 5, 2017
@maumar
Fix to #7364 - Query : ToString() method that takes a formatter argum…
7cc8ce2 
…ent should still be translated on the client

Fix is to only translate ToString calls that have no arguments. Ones with the arguments will be evaluated on the client.
maumar added a commit that referenced this issue Jan 6, 2017
@maumar
Fix to #7364 - Distinct() after Select()ing a property doesn't ge…
f0de50c 
…nerate correct SQL if query also contains optional navigation

Problem was that optional navigation is translated to GroupJoin which in turn forces materialization on entire entity. This causes Distinct to be applied on all columns of that entity, even if the customer specified a subset of columns to perform Distinct on.
Fix is to always perform Distinct on the client if the query also contains GroupJoin
maumar added a commit that referenced this issue Jan 6, 2017
@maumar
Fix to #7364 - Distinct() after Select()ing a property doesn't ge…
0859c4e 
…nerate correct SQL if query also contains optional navigation

Problem was that optional navigation is translated to GroupJoin which in turn forces materialization on entire entity. This causes Distinct to be applied on all columns of that entity, even if the customer specified a subset of columns to perform Distinct on.
Fix is to always perform Distinct on the client if the query also contains GroupJoin
@divega divega added this to the 2.0.0 milestone Jan 9, 2017
@divega
Copy link
Contributor

divega commented Jan 9, 2017

We could consider actually supporting translation in some specific cases.

maumar added a commit that referenced this issue Jan 11, 2017
@maumar
Fix to #7364 - Query : ToString() method that takes a formatter argum…
8b0ef7f 
…ent should still be translated on the client

Fix is to only translate ToString calls that have no arguments. Ones with the arguments will be evaluated on the client.
@maumar
Copy link
Contributor Author

maumar commented Jan 11, 2017

change translating ToString with formatter to the client has been checked in here: 8b0ef7f

@smitpatel
Copy link
Contributor

@maumar - Is this fixed?

@maumar
Copy link
Contributor Author

maumar commented Jan 20, 2017

partially: the part about ignoring formatter is fixed (i.e. now we do client evaluation rather than ignoring the formatter), but what @divega was suggesting (i.e. translate some of the formatters to SQL) is not.

@smitpatel smitpatel changed the title Query : ToString() method that takes a formatter argument should still be translated on the client Query : ToString() method that takes a formatter argument should translated on the server Jan 20, 2017
@divega
Copy link
Contributor

divega commented Jan 20, 2017

@maumar I think that was a note I took in triage but the original suggestion came from @anpete. If we can take a quick look and what formatters would make a lot of sense to translate then I think we can create a new issue for that and prioritize it in triage.

@smitpatel
Copy link
Contributor

We need to do analysis on formatters here. There are multiple formatters for each type, with different set of formatters on client/server side.

@smitpatel
Copy link
Contributor

@divega - Can you provide specific set of formatters which we want translated?

@divega
Copy link
Contributor

divega commented Mar 24, 2017 •
edited
Loading

@smitpatel Here is the link to the spec: https://github.com/aspnet/EntityFramework/wiki/it-does-not-work-like-that

:trollface:

Actually, I am glad that you ask because it forces me to think 😄 I believe this issue should have been closed as soon as we took @maumar's fix to switch to client evaluation rather than ignoring the formatters. We can always look into translating specific formatters to SQL based on customer data, but we don't have any at the moment.

Clearing up the milestone so we can make a final decision in triage.

@divega divega removed this from the 2.0.0 milestone Mar 24, 2017
@ajcvickers
Copy link
Member

Ensure that we are doing client eval. Wait for feedback before deciding to do any translation of formatters.

@ajcvickers ajcvickers added this to the 2.0.0 milestone Mar 27, 2017
@smitpatel
Copy link
Contributor

We are doing client eval already. @maumar 's PR fixed it. Looks like we forgot to close the issue earlier.

@smitpatel smitpatel changed the title Query : ToString() method that takes a formatter argument should translated on the server Query : ToString() method that takes a formatter argument should do client evaluation May 9, 2017
@divega divega added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. labels May 10, 2017
This was referenced Nov 6, 2017
Closed
Closed
@los93sol
Copy link

los93sol commented Nov 7, 2017

In my specific issue #10227, evaluating on the client does not resolve the issue for me

@los93sol
Copy link

los93sol commented Nov 7, 2017

For clarity, when and logic is applied to the where then doing this on the client is fine, but when we or the logic together with other fields you have a potential for unexpected results

@smitpatel
Copy link
Contributor

@los93sol - Client evaluation does not work by breaking into pieces without consequences. It will translate where clause appropritately & evaluate on in memory whatever cannot and always give correct results regardless of AND/OR logic. If you are having a repro code which demonstrate incorrect results for client evaluation then please file a new issue.

@smitpatel smitpatel mentioned this issue Dec 19, 2018
Closed
@ajcvickers ajcvickers modified the milestones: 2.0.0-preview1, 2.0.0 Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@smitpatel smitpatel

Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Milestone
2.0.0
Development

No branches or pull requests
5 participants
@divega @ajcvickers @smitpatel @maumar @los93sol