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

Warn for LastOrDefault without OrderBy #10493

Closed
julielerman opened this issue Dec 6, 2017 · 8 comments
Closed

Warn for LastOrDefault without OrderBy #10493

julielerman opened this issue Dec 6, 2017 · 8 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@julielerman
Copy link

LastOrDefault is still being evaluated locally and spits out a warning about it at runtime:
The LINQ expression LastOrDefault could not be translated and will be evaluated locally.
I'm not seeing evidence (searching in this repo, or listed on the roadmap) that it's on target to get implemented.

Any news?

@smitpatel
Copy link
Contributor

Can you post the query?

@julielerman
Copy link
Author

Sure, I've tried multiple versions:
var samurai=_context.Samurais.LastOrDefault();
That results in
SELECT [s].[Id], [s].[Name]
FROM [Samurais] AS [s]

var samurai=_context.Samurais.LastOrDefault(s=>s.Name=="Sampson");
and
var samurai = _context.Samurais.Where(s=>s.Name=="Sampson").LastOrDefault();

result in:
SELECT [s].[Id], [s].[Name]
FROM [Samurais] AS [s]
WHERE [s].[Name] = N'Sampson'

@julielerman
Copy link
Author

p.s. the warning message is a result of me reporting this a year ago: #6929

@smitpatel
Copy link
Contributor

smitpatel commented Dec 6, 2017

Lack of order by is the cause of client eval.
For particular case of LastOrDefault, we invert the ordering for SelectExpression and do Top(1). Since ordering is not present, LastOrDefault client evaluated.
Further, the results of above query are somewhat non-deterministic. The result could vary based on sorting database provider does. SqlServer records are sorted by PK values but the same thing is not true in Postgre. We have warning system for queries with such non-deterministic results but seems like it is buggy.

https://github.com/aspnet/EntityFrameworkCore/blob/b6aed320cc805aaf7449912ad87f674b4879ec66/src/EFCore/Query/EntityQueryModelVisitor.cs#L412-L428

We are checking for Skip/Take/First. We should also issue warning for Last. (Single can be excluded since ordering may not matter)

@julielerman
Copy link
Author

That makes a lot of sense. I'm just testing the methods so doing random things and in this case, I can now see that my randomness is illogical.
Perhaps the warning message could be clearer as to WHY the method couldn't be translated..that it requires some type of ordering etc .This would also be an EXCELLENT case for some type of compiler warning.

@ajcvickers ajcvickers changed the title Evaluate LastOrDefault in database, not in memory Warn for LastOrDefault without OrderBy Dec 6, 2017
@ajcvickers
Copy link
Contributor

Updating the issue to track adding the OrderBy warning for Last/LastOrDefault/etc.

@ajcvickers ajcvickers added this to the 2.1.0 milestone Dec 6, 2017
@julielerman
Copy link
Author

new title makes sense. thanks

@ajcvickers ajcvickers modified the milestones: 2.1.0-preview1, 2.1.0 Jan 17, 2018
@anpete anpete closed this as completed in 18ba9ad Jan 31, 2018
@divega divega added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 7, 2018
@JoasE
Copy link

JoasE commented Oct 3, 2019

In:
EF Core version: 3.0.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer 3.0.0

I don't get a clear message the exception is being caused by a missing OrderBy, just a message saying:

'The LINQ expression 'LastOrDefault<Nullable<Guid>>(Select<Comment, Nullable<Guid>>(
    source: Where<Comment>(
        source: DbSet<Comment>, 
        predicate: (c) => Property<Nullable<Guid>>(EntityShaperExpression: 
            EntityType: Post
            ValueBufferExpression: 
                ProjectionBindingExpression: EmptyProjectionMember
            IsNullable: False
        , "Id") == Property<Nullable<Guid>>(c, "PostId")), 
    selector: (c) => Property<Nullable<Guid>>(c, "Id")))' could not be translated. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to either AsEnumerable(), AsAsyncEnumerable(), ToList(), or ToListAsync(). See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.'

when executing this query:

var manualQuery = context.Users.Select(dtoUser => new User.ViewDto
{
    Id = dtoUser.Id,
    Posts = dtoUser.Posts
        .Select(
            dtoPost => new Object_205617611___LastComment_Id_Text_UserId
            {
                __LastComment = dtoPost.Comments.LastOrDefault(),
                Id = dtoPost.Id,
                Text = dtoPost.Text,
                UserId = dtoPost.UserId
            })
        .Select(
            dtoLet => new Post.ViewDto
            {
                Id = dtoLet.Id,
                LastComment = (dtoLet.__LastComment == null)
                    ? null
                    : new Comment.ViewDto
                    {
                        Id = dtoLet.__LastComment.Id,
                        PostId = dtoLet.__LastComment.PostId,
                        Text = dtoLet.__LastComment.Text
                    },
                Text = dtoLet.Text,
                UserId = dtoLet.UserId
            })
        .ToList()
}).FirstOrDefault(x => x.Id == user.Id);

Can this issue be re-opened for 3.0.0 or do I have to create a new issue?

Reproduce repo: https://github.com/JoasE/AutoMapperRepro

Moved from: #14991

@JoasE JoasE unassigned anpete Oct 3, 2019
@ajcvickers ajcvickers modified the milestones: 2.1.0-preview2, 2.1.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

No branches or pull requests

6 participants