Add support for table valued functions

[pmiddleton]

This is still a wip. I'm putting this out here in response to @divega inquiry on #4319

This adds support for the following items

• Bootstrapable Scalar Functions
• Bootstrapable TVFs
• Nestable DbFunction calls on both types of bootstrapped methods (pass a function as a parameter to the bootstrapped function)
• Full query support for TVFs via Cross Join
• TVF integration with Query Types. You can either directly materialize the QT from the TVF, or use it as part of a larger query.

I am still working on Outer Apply support.

The code is not ready for a full review at this time. There are a ton of nits and what not that need to be cleaned up still.

I am placing this PR here so the team can see the general approach I have taken and determine if there are any breaking changes which would preclude this from getting into 2.1

Feedback is welcome as always.

[pmiddleton]

@divega - I'm assuming we want to support a query like this? GetCustomerOrderCountByYear is a table valued function in this example.

from c in context.Customers
select new
{
     c.Id,
     Orders = context.GetCustomerOrderCountByYear(c.Id).ToList()
}

[ajcvickers]

@pmiddleton Thanks for all your work on this. Given where we are for 2.1, we're going to push this out to the next release--we're still very interested in getting this done, we just don't want to destabilize 2.1. Assigning to @divega for further follow up.

[pmiddleton]

@ajcvickers

No problem - I knew I was late to the game for 2.1. I hit a roadblock using TVF in select projections last week that has slowed me down. This will give me time to figure out how to implement that.

[divega]

I'm assuming we want to support a query like this?

@pmiddleton sorry for the delay in answering. Yes, seems compelling. But if enabling that is too expensive we can scope it out.

[pmiddleton]

@divega - Sounds good. I worked on it over the weekend and I believe I have found an approach that will work. I just need to finish fleshing it out.

[pmiddleton]

@divega @ajcvickers @smitpatel @anpete

This PR is ready for review. Here is how to use the features.

Registering a TVF

TVFs are registered using the same APIs as scalar functions. TVFs must meet the following conditions.

1. It must be a member function on the DbContext (no static methods).
2. It must return an IQueryable where T is either an EntityType or QueryType.
3. It must call and return the base class method Execute.

public IQueryable<BlogPost> GetTopBlogPosts()
{
      return Execute<BlogContext, BlogPost>(db => db.GetTopBlogPosts());
}

Querying

Once registered the TVF can be used as a data source. It can either be queried directly or be used as part of a larger query.

from b in context.GetTopBlogPosts()
orderby b.PostDate
select b

If used in a larger query the default action will be to perform a Cross Apply.

from u in users
     b in context.GetTopBlogPostsForUser(u.id)
     select new 
     {
         User = u,
         Blog = b
     }

You can use an Outer Apply if you put a DefaultIfEmpty() call onto the TVF.

from u in users
     b in context.GetTopBlogPostsForUser(u.id).DefaultIfEmpty()
     select new 
     {
         User = u,
         Blog = b
     }

You can also perform inner and left joins if your TFV is noncorrelated.

TVFs can also be used as subqueries inside of select clauses.

from c in context.Customers
select new
{
    c.Id,
    Blogs = context.GetTopBlogPostsForUser(c.Id).ToList()
})

Bootstrap scalar functions

The code which allows us to bootstrap TVFs also allows us to now call scalar functions directly. The scalar functions are registered normally, but now they must call and return the base class Execute method.

public string SchemaName()
{
   return Execute<BlogContext, string>(db => db.SchemaName());
}

var schemaName = context.SchemaName();

Nested function calls on bootstraped functions

If you need to pass a function as a parameter on a bootstrapped function you need to define the method parameter as an Expression, and pass the function as a lambda. This is done to prevent the compiler from trying to execute the nested function call.

public int GetBlogPostCountForUser(Expression<Func<int>> userId)
{
     return Execute<BlogContext, i.nt>(db => db.GetBlogPostCountForUser(userId));
}

var value = context.GetTopBlogPostsForUser(() => context.GetTopPosterId());

One area of the design I am still questioning is the Execute method on DbContext.

In order to get ReLinq to parse a function as a data source we need ReLinq to replace the method call with a custom expression object. In the Execute method we also need to be able to create this expression, therefore I had to introduce a factory into the EFCore project. This introduces a db function concept into the Core project which we were trying to avoid.

One alternative would be to define Execute as an extension method on DbContext in the Relational project. This has two drawbacks. The first is that Execute then has to be a public method instead of protected. This is probably not a method we want to expose directly. The second issue is that Execute needs access to the private DbContextDependencies property on DbContext. This means we would have to use reflection to access it, which leads to somewhat brittle code moving forward.

If anyone has any other thoughts on how to do this please let me know.

[divega]

@ajcvickers we can as well decide that postponing to 3.0 is the right thing to do, but perhaps we will know better how this interacts with #12795 if @smitpatel takes a look at the PR.

[ajcvickers]

@smitpatel Your thoughts on @divega's comments?

[smitpatel]

The PR looks alright. Though I can see some of the efforts could get wasted based on outcome of #12795 especially w.r.t. ReLinq change.

[pmiddleton]

@smitpatel - Do you have a plan for #12795 yet or are you still coming up with ideas?

[smitpatel]

@pmiddleton - Still evaluating few ideas. The major thing for that issue to be in 3.0 is ReLinq. See (#12048). Which would be major breaking change and significant architectural change.

[pmiddleton]

Rebased to master

[pmiddleton]

@dotnet-bot test windows release x64 build

[divega]

@pmiddleton sorry for the lack of communication on this lately. I just want to confirm that we discussed this PR based in light of the architectural changes we are planning for the query pipeline in 3.0 and we came to the conclusion that we want to postpone introducing the feature until those are complete.

As usual, you have done a great job on this PR and we look forward to collaborate with you to adjust it in 3.0. For the time being I am going to assign this to the 3.0 milestone.

[divega]

Blocked on #12795.

[pmiddleton]

@divega - This make sense. I look forward to seeing the new approach in 3.0 and reworking this pr.

Any idea what a time frame is for a first look at the new approach? I have some ideas for changes on how we deal with functions in general moving forward.

[divega]

@pmiddleton not sure when we will start seeing more of the new approach. The team is pretty busy locking down 2.2 at the moment. cc @smitpatel

[agevorg]

Hi @divega ,
When we can expect this feature to be delivered? It was due to 2.0, than 2.1, than 2.2 now it will not make to 3.0.

[divega]

@agevorg so far we have relied on the exceptional contributions from @pmiddleton to have advanced function support in EF Core. Now that the the new LINQ pipeline is starting to settle, in theory it should be possible to resume work on this. But to be completely honest, if @pmiddleton doesn't have the time to do it now, it is unlikely that it will be included in EF Core 3.0.

cc @smitpatel @ajcvickers

[pmiddleton]

Unfortunately I won't have time to get this done for 3.0. My family needed to buy a house and move this spring/summer so I haven't had any free time to work on this. Since 3.0 rewrote the entire query pipeline this feature will need to be mostly rewritten from the ground up as well, it won't be a simple port.

My goal is to get this done for the next major release in 2020.

[smitpatel]

Moving this out of 3.0 milestone

[pmiddleton]

Doh - stupid lazy developer... oh wait 😁

[smitpatel]

I am closing this PR as per last communication. It is true that it would almost rewrite rather than just rebase or port given the state of new query pipeline. GitHub would preserve the discussion if we need to reference in future.