Client vs. Server evaluation can sometimes call a DbFunction's static method template

[JonPSmith]

I had the exception inside one of my [DbFunction] static method template being triggered in a complex query. I stripped everything away, and I found that an outer OrderBy was deemed to need Client vs. Server evaluation, which dragged the DbFunction method into the client side evaluation, with the obvious result of exception in the static method being called.

I found this type of error quite hard to diagnose, as it isn't obvious what is happening. I really didn't think a DbFunction would be be called in Client vs. Server evaluation, but now I know.

I have isolated a simple example as shown below

System.Exception : Exception of type 'System.Exception' was thrown.
   at DataLayer.SqlCode.UdfDefinitions.AuthorsStringUdf(Int32 bookId) in C:\Users\Jon\Documents\Visual Studio 2017\Projects\EfCoreInAction\DataLayer\SqlCode\UdfDefinitions.cs:line 33
   at lambda_method(Closure , ValueBuffer )
   at System.Linq.EnumerableSorter`2.ComputeKeys(TElement[] elements, Int32 count)
   at System.Linq.EnumerableSorter`1.ComputeMap(TElement[] elements, Int32 count)
   at System.Linq.EnumerableSorter`1.Sort(TElement[] elements, Int32 count)
   at System.Linq.OrderedEnumerable`1.<GetEnumerator>d__3.MoveNext()
   at System.Linq.Enumerable.SelectIPartitionIterator`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Collections.Generic.List`1.AddEnumerable(IEnumerable`1 enumerable)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Test.UnitTests.ServiceLayer.Ch13_BookListDtoImproved.TestUdfExceptionSimpleDto() in C:\Users\Jon\Documents\Visual Studio 2017\Projects\EfCoreInAction\Test\UnitTests\ServiceLayer\Ch13_BookListDtoImproved.cs:line 104

The logging output from my unit test is

Warning: The LINQ expression 'orderby new SimpleDto() {AuthorsOrdered = AuthorsStringUdf([b].BookId)}.ReviewsAverageVotes asc' could not be translated and will be evaluated locally.

Information: Executed DbCommand (3ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
SELECT [b].[BookId], [dbo].AuthorsStringUdf([b].[BookId]) AS [AuthorsOrdered]
FROM [Books] AS [b]
WHERE [b].[SoftDeleted] = 0

Steps to reproduce

Below I list my unit test. The significant thing in this unit test is that the ReviewsAverageVotes value is not set, which seems to trigger the Client vs. Server evaluation (My query was a lot more complex that that, but I proved it triggered the same issue).

Notes:

• The statement var options = this.ClassUniqueDatabaseSeeded4Books(); produces a uniquely named database and seeds it with four test books.
• The method context.ExecuteScriptFileInTransaction(filepath); adds the UDFs to the database.

private class SimpleDto
{
    public double? ReviewsAverageVotes { get; set; }
    public string AuthorsOrdered { get; set; }
}

[Fact] public void TestUdfExceptionSimpleDto()
{
    //SETUP
    var options = this.ClassUniqueDatabaseSeeded4Books();
    var filepath = Path.Combine(TestFileHelpers.GetSolutionDirectory(),
        @"EfCoreInAction\wwwroot\",
        UdfDefinitions.SqlScriptName);

    using (var context = new EfCoreContext(options))
    {
        context.Database.EnsureCreated();
        context.ExecuteScriptFileInTransaction(filepath);
        var logIt = new LogDbContext(context);

        //ATTEMPT
        var books = context.Books.Select(p => new SimpleDto
        {
            AuthorsOrdered = UdfDefinitions.AuthorsStringUdf(p.BookId)
        }).OrderBy(x => x.ReviewsAverageVotes).ToList();

        //VERIFY
        books.Select(x => x.AuthorsOrdered).ToArray()
            .ShouldEqual(new string[] { "Martin Fowler", "Martin Fowler", "Eric Evans", "Future Person" });
        foreach (var log in logIt.Logs)
        {
            _output.WriteLine(log);
        }
    }
}

My entity classes (which you must be sick of seeing:)) are:

public class Book                                   
{
    public int BookId { get; set; } 
    public string Title { get; set; }
    public string Description { get; set; }
    public DateTime PublishedOn { get; set; }
    public string Publisher { get; set; }
    public decimal Price { get; set; }
    public string ImageUrl { get; set; }

    //----------------------------------------------
    //relationships

    public PriceOffer Promotion { get; set; }       
    public ICollection<Review> Reviews { get; set; }
    public ICollection<BookAuthor> AuthorsLink { get; set; }                   
}

public class Review
{
    public const int NameLength = 100;

    public int ReviewId { get; set; }
    [MaxLength(NameLength)]
    public string VoterName { get; set; }
    public int NumStars { get; set; }
    public string Comment { get; set; }

    //-----------------------------------
    //Relationships

    public int BookId { get; set; }
}

public class Author                   
{
    public int AuthorId { get; set; }
    public string Name { get; set; }

    //------------------------------
    //Relationships

    public ICollection<BookAuthor> 
        BooksLink { get; set; }       
}

Linking Table is

public class BookAuthor               
{
    public int BookId { get; set; }   
    public int AuthorId { get; set; } 
    public byte Order { get; set; }   

    //-----------------------------
    //Relationships

    public Book Book { get; set; }    
    public Author Author { get; set; }
}

I haven't provided the DbContext with its setup, but I don't think you need it. If you do then I can set it, but all the code can be found the open-source project on GitHub in branch Chapter13-Part1 - see https://github.com/JonPSmith/EfCoreInAction/tree/Chapter13-Part1. You can find the actual unit test at https://github.com/JonPSmith/EfCoreInAction/blob/Chapter13-Part1/Test/UnitTests/ServiceLayer/Ch13_BookListDtoImproved.cs#L83-L117 .

Further technical details

EF Core version: EF Core 2.0.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10
IDE: Visual Studio 2017 15.3

[smitpatel]

dbug: Microsoft.EntityFrameworkCore.Query[100104]                                       
      Optimized query model:                                                            
      'from Book p in DbSet<Book>                                                       
      order by new SimpleDto{ AuthorsOrdered = string AuthorsStringUdf([p].BookId) }    
      .ReviewsAverageVotes asc                                                          
      select new SimpleDto{ AuthorsOrdered = string AuthorsStringUdf([p].BookId) }      
      '                                                                                 

Above is the query model generated for the query.
Few observations, you are setting AuthorsOrdered property in DTO during projection but ordering by a different property. Is it by design?
Since ReLinq generates above querymodel, we cannot generate the ordering. If you are ordering by after Select then you can call ToList after select then order by on client.

[pmiddleton]

It's known that a DbFunction may evaluate client side. As you have found out if the query is not completely translatable the function may be run clientside, depending on what part of the query it is in.

In your case you are ordering on a property that the database doesn't know about, so EF breaks the query into server and client side.

As @smitpatel said to work around this you can either sort on a column in the database, or ToList the select and then apply the order by.

Alternatively you can provide a client side implementation of the DbFunction and it will be run correctly. Not all functions are implementable client side, but some are, and in those cases it will work.

[JonPSmith]

Thanks for the input. As I added to my first comment - I found this type of error quite hard to diagnose, as it isn't obvious what is happening. I didn't even think that a DbFunction would be evaluated client side, but now I know.

I think I will change the Exception in say "Called in Client vs. Server evaluation" (and update the book to say that too).

Thanks for clarifying this.

[smitpatel]

While it may seem bit difficult to figure out what happened there is enough of logging happening for that.

System.Exception : Exception of type 'System.Exception' was thrown.
at DataLayer.SqlCode.UdfDefinitions.AuthorsStringUdf(Int32 bookId)

The line where exception is happening is from DbFunction defined. It is System.Exception because that is what is written in code. Throwing NotImplementedException would provide more clarity.

Warning: The LINQ expression 'orderby new SimpleDto() {AuthorsOrdered = AuthorsStringUdf([b].BookId)}.ReviewsAverageVotes asc' could not be translated and will be evaluated locally.

This capture that the function will be called on client explicitly.

[JonPSmith]

Hi @pmiddleton,

I have been looking at the idea that DbFunction methods could be called by client vs. server evaluation, and therefore could have real code inside the DbFunction method body. My first try was to look at adding code into the static method, but there is no access to the current DbContext (well it could be, but only via some static approach, which is horrible), so that was a non-starter.

I did try defining a non-static method inside the DbContext, which would have allowed the method access to the current DbContext, but I get the error:

System.ArgumentException : The DbFunction 'Chapter08EfCoreContext.AverageVotes' must be a static method. Non-static methods are not supported.

I therefore content that creating a a software version is pretty difficult, as the UDF is likely to take the minimum of information as an input parameter and it cannot access the DbContext. My AverageVotes example is contrived, but I use the DbFunction usefully later in the book and none of them would work as software defined code.

Am I missing something here? If you want to allow client vs. server evaluation is it possible to allow the DbFunction method to be non-static?? I would appreciate your thoughts on this.

PS. Personally I think the DbFunction is the easiest way to include custom SQL in a query and I want to understand this to convey this to others.

[pmiddleton]

Currently you can't define an instance method as a dbFunction. That is a limitation due to time constraint in order to get things done for 2.0.

I am actually working on this feature now - see #9213

I had not considered the use case which you are proposing for that feature. I had always thought it as just a shorthand to avoid some typing. I had not considered making database calls from it in cases of client side evaluation.

IDK if that would work. You would be making one or more database calls in the middle of another parent call. It may work, or it may blow up.

@smitpatel @anpete - care to put forward any guesses what would happen in that use case?

I will give it a try once I can get instance methods working and see what happens. At a minimum it seems very bad performance wise as you will would end up executing N+1 queries.

I always envisioned that the ability to make that client side call would be for simple functions, and actually something you would actively try to avoid in favor of server side execution.

[anpete]

It should work (it's conceptually the same as any other mixed eval query) but we may need to address any wrinkles that crop up.

Some advantages of providing a client implementation are:

1. It facilitates in-memory testing.
2. It allows the query to work if it happens to fallback to client eval before the function call.

[pmiddleton]

@JonPSmith

I have a working copy of instance method support up if you want to play with it.
https://github.com/pmiddleton/EntityFramework/tree/nonStatic

This is based off of the latest dev branch.

I would be curious if it works for the scenario you are envisioning.

[JonPSmith]

Hi @pmiddleton,

I read your comment and I thought I would give you my view on how/why DbFunctions will be used. My comments about developer usage scenarios, but hopefully they may help you in your thinking.

1. I think DbFunction is the best SQL language feature in EF Core. Why? Because EF Core is now producing some beautiful SQL code, but sometimes a developer might need to "tweak" part of the query's SQL. DbFunction allows a developer to add a UDF, which contains a specific snippet of SQL, to any query, including queries to non-entity classes.
2. DbFunctions and client vs. server evaluation: Client vs. server evaluation is a blessing and a curse - it is easy to trip over something and part of the query drops into client-side evaluation mode - in EF6.x that couldn't happen (I know - I can configure client vs. server evaluation to throw an exception, but its not the default setting). However, in line with "develop fast, performance tune later" I think allowing DbFunctions to be called by client vs. server evaluation is a good idea, but I don't see it as the default usage.
3. I really like the new CreateFunction method for defining DbFunctions, as shown by @bricelam in his SQLite & EF Core blog post and I hope it will be available on all database providers. I think it will become the primary way to define DbFunctions. But client vs. server evaluation should return a sensible error if a DbFunction defined with CreateFunction appears in a client-side evaluation.
4. I agree with @bricelam, that the call to the DbFunction is much better if it refers to the method via the context, rather than via a class name, e.g.

db.Posts.Where(p => db.PostReadCount(p.Id) > 5);

5. I have to think about what I am going to say in the book about DbFunctions as the aim is to get the book into the print run in the next two months. I can update the eBook, but not the print book.

I hope my ramblings are helpful. I'm pleased to see DbFunctions are going to evolve in the future. I love them.

[pmiddleton]

@JonPSmith

I ran a test for your use case. I tried making another database call from a dbFunction which was being client side evaluated.

As I thought EF Core won't let this call happen as there is another call in progress. The internal concurrency detection prevents the second call from completing.

Client side evaluation for DB Functions is going to have to stay limited to very simple functions which can be implemented without making any database calls.

In regards to item 3. That is only ever going to be a Sqlite feature. Sqlite UDFs are implemented in C not in SQL. Interop is then allowing .Net code to replace the C function in @bricelam example. For other providers to support this we would need some kind of C# -> Sql translation which is just not possible.

[anpete]

@pmiddleton You are hitting #7375 - we need to fix it.

[pmiddleton]

I will keep an eye on that issue and retest if/when it is resolved.

@JonPSmith even if the re-entrance issue is resolved I still would recommend against coding a method this way as it will still have the N+1 issue. Finding a way to rework the query to run server side is a much better option.

[JonPSmith]

@pmiddleton, I am quite happy with DbFunction not running with client vs. server, and I think others will be happy too. One of the reasons I found it hard to diagnose the problem I couldn't even envisage that a SQL UDF would run client side.

[ajcvickers]

Note from triage: Since using the context inside client eval of the function is blocked by #7375, and this is in turn blocked by #8864, @anpete is going to investigate #8864 again to see whether we can do something for 2.1. However, #8864 looks quite hard, so we will cycle back again after the investigation and decide how to proceed.