EF mistreats LINQ methods chained after FromSql

[gdoron]

I'm getting the following warning using RC-1-Final:

The LINQ expression 'orderby ([x].ReplyToPostId ?? -1) asc, [x].IsImportantReply desc, [x].PostId asc' could not be translated and will be evaluated locally.

Code:

    public Task<List<HierarchyPost>> GetRootAsync(int postId)
    {
        return HierarchyPosts.FromSql(@"
with postsCTE as (
   select Id, ReplyToPostId
   from Posts
   where Id = @p0
   union all
   select Parent.Id, Parent.ReplyToPostId
   from Posts Parent
     join postsCTE Son on Son.ReplyToPostId = Parent.Id  -- this is the recursion
) 
select * from HierarchyPosts where RootId =(
                                            select top 1 Id
                                            from PostsCTE
                                            where ReplyToPostId is null)", postId)
                .OrderBy(x => x.ReplyToPostId ?? -1)
                .ThenByDescending(x => x.IsImportantReply)
                .ThenBy(x => x.PostId)
                .ToListAsync();
    }

It's quite easy to translate the query to SQL (using case) and indeed it worked in earlier versions of EF.

[rowanmiller]

@maumar @anpete is this something we have already addressed in working code base?

[gdoron]

@rowanmiller Let me do some more tests, I'm not sure it indeed because of the ?? operator.
Maybe it's just because of joing FromSql with Lamdas.
I think when I removed the null coalescing operator it worked, but maybe I mixed some of my tests.

I'll update soon.

[gdoron]

@maumar @rowanmiller
Sorry for the extremely late response, my initial bug report was inaccurate (I was in the middle of testing it as my wife decided to give birth... and the cute baby "stole" all my free time).

Anyway, it seems not to be related to the null coalescing operator but EF in general has issues with mixing FromSql with other Linq methods.
For example if I add OrderBy after FromSql, I'll get a warning it will be done locally (even without ??).
If I use Select, it will silently be completely ignored. (doesn't affect the generated SQL nor done on the server).

LMK if you need examples.

[divega]

@gdoron first of all, many congratulations! 😄

FromSql() currently doesn't support CTEs because the way we compose LINQ operators over SQL queries is by turning the latter into derived tables for new queries. If you use a CTE in a derived table or subquery you are supposed to place the CTE before the main SELECT clause not inside the parenthesis alongside the FROM clause!

Your query will still execute and should return correct results with FromSql() but LINQ operators won't be processed on the server because EF Core will correctly detect that the query string is not composable in the conventional way.

We have talked about the possibility of extending the FromSql() feature to accept passing a "preamble" to be prepended to whatever SQL results from the composition precisely to address this scenario, but this isn't implemented and in the meanwhile the best workaround I can think of is to put your CTEs in views that you can then reference in the SQL queries you pass to FromSql().

HTH.

[divega]

Cc @anpete

[gdoron]

@divega Thanks.

I would change the error message to be more understandable it's because of the CTE.
I agree that View would be the way to go here, but since EF Core support for View is by tricking the Migrations and requires tons of hassle, I prefer not to use Views unless there is no other reasable option.

Just worth mentioning, using Select is being completely ignored.

e.g.

public Task<List<HierarchyPost>> GetRootAsync(int postId)
    {
        return HierarchyPosts.FromSql(@"
with postsCTE as (
   select Id, ReplyToPostId
   from Posts
   where Id = @p0
   union all
   select Parent.Id, Parent.ReplyToPostId
   from Posts Parent
     join postsCTE Son on Son.ReplyToPostId = Parent.Id  -- this is the recursion
) 
select * from HierarchyPosts where RootId =(
                                            select top 1 Id
                                            from PostsCTE
                                            where ReplyToPostId is null)", postId)
                .Select(x=> new HierarchyPost
                                        {
                                            Body ="123"
                                            Title = x.Title
                                        })
                .ToListAsync();
    }

Will return the HierarchyPost as they are on the DB.

[divega]

Just worth mentioning, using Select is being completely ignored.

In case I wasn't clear: You SQL query starts with the word "WITH", not with "SELECT", therefore it cannot be pushed down to a FROM clause to form a new query. Hence EF Core should execute you SQL string as is and then apply any additional LINQ operators in memory.

If you are actually seeing that the LINQ operators are not applied at all and the results coming out of ToListAsync() are the same you would get without the LINQ select at the end, that sounds like a bug.

[gdoron]

@divega by ignored I meant the emitted SQL isn't effected by the Select, and the results returned are the same with or without the Select.

[divega]

Ok, thank for reporting that! We will investigate but in the meanwhile a couple of things you can do to help:

• try our latest bits if possible to see if it still fails
• provide a small but complete repro

[gdoron]

@divega here is a simple repro demo:
https://github.com/gdoron/Forums/blob/SelectFromFromSql/src/Forums/Controllers/HomeController.cs#L24-L32

Regarding testing using the latest bits, I'm afraid I currently don't have the time to upgrade to RC2 by searching for fixes for all the breaking changes.
Had there a tutorial showing how to upgrade from RC1 to devci, I would be happy doing so, until then, I think I'll stick with RC1 and when there will be templates for RC2, I'll just use my spectacular copy-paste capabilities and upgrade. 😄

[rowanmiller]

@anpete can you take a look at losing the composed LINQ after non-composable FromSql call for RC2

[anpete]

@rowanmiller Sure.

[anpete]

Confirmed the issue.

[divega]

I have created #4976 to track the idea that should enable CTE composition.