Add support for table valued functions

src/EFCore.Relational/Infrastructure/EntityFrameworkRelationalServicesBuilder.cs

@@ -203,6 +203,7 @@ public override EntityFrameworkServicesBuilder TryAddCoreServices()
                 .AddDependencySingleton<RelationalValueBufferFactoryDependencies>()
                 .AddDependencySingleton<RelationalProjectionExpressionVisitorDependencies>()
                 .AddDependencySingleton<RelationalTransactionFactoryDependencies>()
+                .AddDependencySingleton<RelationalDbFunctionSourceFactoryDependencies>()
                 .AddDependencyScoped<RelationalConventionSetBuilderDependencies>()
                 .AddDependencyScoped<CommandBatchPreparerDependencies>()
                 .AddDependencyScoped<RelationalDatabaseCreatorDependencies>()

src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs

@@ -80,8 +80,8 @@ protected virtual void ValidateDbFunctions([NotNull] IModel model)
 
                 if (dbFunction.Translation == null)
                 {
-                    if ((dbFunction.IsIQueryable && model.FindEntityType(dbFunction.MethodInfo.ReturnType.GetGenericArguments()[0]) == null)
-                        && RelationalDependencies.TypeMappingSource.FindMapping(methodInfo.ReturnType) == null)
+                    if (dbFunction.IsIQueryable && model.FindEntityType(dbFunction.MethodInfo.ReturnType.GetGenericArguments()[0]) == null
+                        || !dbFunction.IsIQueryable && RelationalDependencies.TypeMappingSource.FindMapping(methodInfo.ReturnType) == null)
                     {
                         throw new InvalidOperationException(
                             RelationalStrings.DbFunctionInvalidReturnType(

src/EFCore.Relational/Metadata/IDbFunction.cs

@@ -29,7 +29,7 @@ public interface IDbFunction
         MethodInfo MethodInfo { get; }
 
         /// <summary>
-        ///     Does this method return IQueryable
+        ///     Whether this method returns IQueryable
         /// </summary>
         bool IsIQueryable { get; }
 

src/EFCore.Relational/Query/ExpressionVisitors/RelationalEntityQueryableExpressionVisitor.cs

@@ -148,6 +148,8 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
         /// <returns> An expression to use in place of the node. </returns>
         protected override Expression VisitExtension(Expression node)
         {
+            Check.NotNull(node, nameof(node));
+
             switch (node)
             {
                 case DbFunctionSourceExpression dbNode:
@@ -264,13 +266,15 @@ var useQueryComposition
                 Expression.Constant(shaper));
         }
 
-         /// <summary>
-        /// todo
+        /// <summary>
+        /// Visit a <see cref="DbFunctionSourceExpression"/> node.
         /// </summary>
-        /// <param name="dbFunctionSourceExpression">todo</param>
-        /// <returns>todo</returns>
-        protected Expression VisitDbFunctionSourceExpression([NotNull] DbFunctionSourceExpression dbFunctionSourceExpression)
+        /// <param name="dbFunctionSourceExpression"> The node being visited. </param>
+        /// <returns> An Exprssion corresponding to the translated  DbFunctionSourceExpression. </returns>
+        protected virtual Expression VisitDbFunctionSourceExpression(DbFunctionSourceExpression dbFunctionSourceExpression)
         {
+            Check.NotNull(dbFunctionSourceExpression, nameof(dbFunctionSourceExpression));
+
             var relationalQueryCompilationContext = QueryModelVisitor.QueryCompilationContext;
             var selectExpression = _selectExpressionFactory.Create(relationalQueryCompilationContext);
 
@@ -306,15 +310,14 @@ var tableAlias
             }
 
             return Expression.Call(
-                QueryModelVisitor.QueryCompilationContext.QueryMethodProvider // TODO: Don't use ShapedQuery when projecting
+                QueryModelVisitor.QueryCompilationContext.QueryMethodProvider 
                     .ShapedQueryMethod
                     .MakeGenericMethod(shaper.Type),
                 EntityQueryModelVisitor.QueryContextParameter,
                 Expression.Constant(_shaperCommandContextFactory.Create(querySqlGeneratorFunc)),
                 Expression.Constant(shaper));
         }
 
-
         private Shaper CreateShaper(Type elementType, IEntityType entityType, SelectExpression selectExpression)
         {
             Shaper shaper;

src/EFCore.Relational/Query/ExpressionVisitors/RelationalEntityQueryableExpressionVisitorDependencies.cs

@@ -47,7 +47,7 @@ public sealed class RelationalEntityQueryableExpressionVisitorDependencies
         /// <param name="selectExpressionFactory"> The select expression factory. </param>
         /// <param name="materializerFactory"> The materializer factory. </param>
         /// <param name="shaperCommandContextFactory"> The shaper command context factory. </param>
-        /// <param name="sqlTranslatingExpressionVisitorFactory"> TODO. </param>
+        /// <param name="sqlTranslatingExpressionVisitorFactory"> The sql translating expression factory. </param>
         public RelationalEntityQueryableExpressionVisitorDependencies(
             [NotNull] IModel model,
             [NotNull] ISelectExpressionFactory selectExpressionFactory,
@@ -89,11 +89,9 @@ public RelationalEntityQueryableExpressionVisitorDependencies(
         public IShaperCommandContextFactory ShaperCommandContextFactory { get; }
 
         /// <summary>
-        /// todo
+        ///     The sql translating expression factory.
         /// </summary>
         public ISqlTranslatingExpressionVisitorFactory SqlTranslatingExpressionVisitorFactory { get; }
-
-
 
         /// <summary>
         ///     Clones this dependency parameter object with one service replaced.

src/EFCore.Relational/Query/ExpressionVisitors/SqlTranslatingExpressionVisitor.cs

@@ -1128,14 +1128,7 @@ var equalityExpression
 
                     var newArguments = Visit(dbFunctionExpression.Arguments);
 
-                    if (newArguments.Any(a => a == null))
-                    {
-                        return null;
-                    }
-
-                    //TODO - can you custom translate here?
-                    return //dbFunctionExpression.Translate(newArguments)
-                            //??
+                    return dbFunctionExpression.Translation?.Invoke(newArguments) ??
                             new SqlFunctionExpression(dbFunctionExpression.Name, dbFunctionExpression.UnwrappedType, dbFunctionExpression.Schema, newArguments);
 
                 case DiscriminatorPredicateExpression discriminatorPredicateExpression:

src/EFCore.Relational/Query/Expressions/DbFunctionSourceExpression.cs

@@ -3,44 +3,48 @@
 using System.Collections.ObjectModel;
 using System.Linq;
 using System.Linq.Expressions;
-using System.Text;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal;
+using Microsoft.EntityFrameworkCore.Utilities;
 
 namespace Microsoft.EntityFrameworkCore.Query.Expressions
 {
     /// <summary>
-    ///     This API supports the Entity Framework Core infrastructure and is not intended to be used 
-    ///     directly from your code. This API may change or be removed in future releases.
+    ///    Represents a Db Function which acts as a query source in the ReLinq parse tree.
     /// </summary>
     public class DbFunctionSourceExpression : Expression
     {
         private readonly IDbFunction _dbFunction;
 
         /// <summary>
-        /// todo
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used 
+        ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
         public override ExpressionType NodeType => ExpressionType.Extension;
 
         /// <summary>
-        /// todo
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used 
+        ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
         public override Type Type { get; }
 
         /// <summary>
-        /// todo
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used 
+        ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
         public virtual Type ReturnType { get; }
 
         /// <summary>
-        /// todo
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used 
+        ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
         public virtual string Schema => _dbFunction.Schema;
 
         /// <summary>
-        /// todo
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used 
+        ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
         public virtual Type UnwrappedType => Type.IsGenericType ? Type.GetGenericArguments()[0] : Type;
 
@@ -51,7 +55,8 @@ public class DbFunctionSourceExpression : Expression
         public virtual string Name => _dbFunction.FunctionName;
 
         /// <summary>
-        /// todo
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used 
+        ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
         public virtual bool IsIQueryable => _dbFunction.IsIQueryable;
 
@@ -62,25 +67,31 @@ public class DbFunctionSourceExpression : Expression
         public virtual ReadOnlyCollection<Expression> Arguments { get; [param: NotNull] set; }
 
         /// <summary>
-        /// todo
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used 
+        ///     directly from your code. This API may change or be removed in future releases.
+        /// </summary>
+        public virtual Func<IReadOnlyCollection<Expression>, Expression> Translation => _dbFunction.Translation ;
+
+        /// <summary>
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used 
+        ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
-        /// <param name="expression">todo</param>
-        /// <param name="model">todo</param>
         public DbFunctionSourceExpression([NotNull] MethodCallExpression expression, [NotNull] IModel model)
         {
+            Check.NotNull(expression, nameof(expression));
+            Check.NotNull(model, nameof(model));
+
             _dbFunction = FindDbFunction(expression, model);
             Arguments = expression.Arguments;
 
             if (expression.Method.ReturnType.IsGenericType)
             {
-                //todo - add unit test
                 if (expression.Method.ReturnType.GetGenericTypeDefinition() != typeof(IQueryable<>))
                 {
                     throw new InvalidOperationException(
                         RelationalStrings.DbFunctionTableValuedFunctionMustReturnIQueryable(_dbFunction.FunctionName));
                 }
 
-                //todo - should i be using the dbfunction return type here?  If not do I have to verify the expression return type?
                 Type = expression.Method.ReturnType;
                 ReturnType = expression.Method.ReturnType.GetGenericArguments()[0];
             }
@@ -97,19 +108,23 @@ public DbFunctionSourceExpression([NotNull] MethodCallExpression expression, [No
         /// </summary>
         public DbFunctionSourceExpression([NotNull] DbFunctionSourceExpression oldFuncExpression, [NotNull] ReadOnlyCollection<Expression> newArguments)
         {
+            Check.NotNull(oldFuncExpression, nameof(oldFuncExpression));
+            Check.NotNull(newArguments, nameof(newArguments));
+
             Arguments = new ReadOnlyCollection<Expression>(newArguments);
             _dbFunction = oldFuncExpression._dbFunction;
             ReturnType = oldFuncExpression.ReturnType;
             Type = oldFuncExpression.Type;
         }
 
         /// <summary>
-        /// todo
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used 
+        ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
-        /// <param name="visitor">todo</param>
-        /// <returns>todo</returns>
         protected override Expression VisitChildren(ExpressionVisitor visitor)
         {
+            Check.NotNull(visitor, nameof(visitor));
+
             var newArguments = visitor.Visit(Arguments);
 
             if (visitor is ParameterExtractingExpressionVisitor)
@@ -132,7 +147,6 @@ private IDbFunction FindDbFunction(MethodCallExpression exp, IModel model)
 
             var dbFunction = model.Relational().FindDbFunction(method);
 
-            //todo - add unit test
             if (dbFunction == null)
             {
                 throw new InvalidOperationException(

src/EFCore.Relational/Query/Expressions/SelectExpression.cs

@@ -1064,8 +1064,6 @@ public virtual JoinExpressionBase AddCrossJoinLateralOuter(
             Check.NotNull(tableExpression, nameof(tableExpression));
             Check.NotNull(projection, nameof(projection));
 
-            //todo - this seems very wrong - where is the right place to do this?  By the caller?  By the sql walker?
-            //for TVF we need to unwrap the inner select clause
             if (tableExpression is SelectExpression s && s.Tables.First() is TableValuedSqlFunctionExpression)
             {
                 tableExpression = s.Tables.First();

src/EFCore.Relational/Query/Expressions/TableValuedSqlFunctionExpression.cs

@@ -3,7 +3,6 @@
 using System.Collections.ObjectModel;
 using System.Linq;
 using System.Linq.Expressions;
-using System.Text;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Query.Sql;
 using Microsoft.EntityFrameworkCore.Utilities;
@@ -12,65 +11,64 @@
 namespace Microsoft.EntityFrameworkCore.Query.Expressions
 {
     /// <summary>
-    ///     Represents a SQL Table Valued Fuction.
+    ///     Represents a SQL Table Valued Fuction in the sql generation tree.
     /// </summary>
     public class TableValuedSqlFunctionExpression : TableExpressionBase
     {
-        private SqlFunctionExpression _sqlFunctionExpression;
+        private readonly SqlFunctionExpression _sqlFunctionExpression;
 
         /// <summary>
-        /// todo
+        /// The sql function expression representing the database function
         /// </summary>
         public virtual SqlFunctionExpression SqlFunctionExpression => _sqlFunctionExpression;
 
         /// <summary>
-        /// todo
+        /// Creates a new instance of a TableValuedSqlFunctionExpression.
         /// </summary>
-        /// <param name="sqlFunction">todo</param>
-        /// <param name="querySource">todo</param>
-        /// <param name="alias">todo</param>
+        /// <param name="sqlFunction"> The sqlFunctionExprssion representing the database function. </param>
+        /// <param name="querySource">  The query source. </param>
+        /// <param name="alias"> The alias. </param>
         public TableValuedSqlFunctionExpression([NotNull] SqlFunctionExpression sqlFunction, [NotNull] IQuerySource querySource, [CanBeNull] string alias)
              : this(sqlFunction.FunctionName, sqlFunction.Type, sqlFunction.Schema, sqlFunction.Arguments, querySource, alias)
         {
-
         }
 
         /// <summary>
-        /// todo
+        /// Creates a new instance of a TableValuedSqlFunctionExpression.
         /// </summary>
-        /// <param name="functionName">todo</param>
-        /// <param name="returnType">todo</param>
-        /// <param name="schema">todo</param>
-        /// <param name="arguments">todo</param>
-        /// <param name="querySource">todo</param>
-        /// <param name="alias">todo</param>
+        /// <param name="functionName"> The db function name. </param>
+        /// <param name="returnType"> The db function return type. </param>
+        /// <param name="schema"> The schema. </param>
+        /// <param name="arguments"> The arguemnts to the db function. </param>
+        /// <param name="querySource"> The query source. </param>
+        /// <param name="alias"> The alias. </param>
         public TableValuedSqlFunctionExpression([NotNull] string functionName,
                 [NotNull] Type returnType,
                 [CanBeNull] string schema,
                 [NotNull] IEnumerable<Expression> arguments,
                 [NotNull] IQuerySource querySource,
-                [CanBeNull]string alias)
-            : base(querySource, alias)
+                [CanBeNull] string alias)
+            : base(Check.NotNull(querySource, nameof(querySource)), alias)
         {
-            //TODO - make sure return type is of type IQueryable<T>
-            //TODO - Do I even need this class or can I just use the SqlFunctionExpression?  Thus far not much is happening in here
+            Check.NotNull(functionName, nameof(functionName));
+            Check.NotNull(returnType, nameof(returnType));
+            Check.NotNull(arguments, nameof(arguments));
+
             _sqlFunctionExpression = new SqlFunctionExpression(functionName, returnType, schema, arguments);
         }
 
         /// <summary>
-        /// todo
+        ///   Convert this object into a string representation.
         /// </summary>
-        /// <returns>todo</returns>
+        /// <returns> A string that represents this object. </returns>
         public override string ToString()
         {
             return _sqlFunctionExpression.ToString();
         }
 
         /// <summary>
-        /// todo
+        /// Dispatches to the specific visit method for this node type.
         /// </summary>
-        /// <param name="visitor">todo</param>
-        /// <returns>todo</returns>
         protected override Expression Accept(ExpressionVisitor visitor)
         {
             Check.NotNull(visitor, nameof(visitor));
@@ -81,15 +79,25 @@ protected override Expression Accept(ExpressionVisitor visitor)
         }
 
         /// <summary>
-        /// todo
+        ///     Reduces the node and then calls the <see cref="ExpressionVisitor.Visit(Expression)" /> method passing the
+        ///     reduced expression.
+        ///     Throws an exception if the node isn't reducible.
         /// </summary>
-        /// <param name="visitor">todo</param>
-        /// <returns>todo</returns>
+        /// <param name="visitor"> An instance of <see cref="ExpressionVisitor" />. </param>
+        /// <returns> The expression being visited, or an expression which should replace it in the tree. </returns>
+        /// <remarks>
+        ///     Override this method to provide logic to walk the node's children.
+        ///     A typical implementation will call visitor.Visit on each of its
+        ///     children, and if any of them change, should return a new copy of
+        ///     itself with the modified children.
+        /// </remarks>
         protected override Expression VisitChildren(ExpressionVisitor visitor)
         {
+            Check.NotNull(visitor, nameof(visitor));
+
             var newArguments = visitor.Visit(new ReadOnlyCollection<Expression>(_sqlFunctionExpression.Arguments.ToList()));
 
-            return newArguments != _sqlFunctionExpression.Arguments
+            return !Equals(newArguments, _sqlFunctionExpression.Arguments)
                 ? new TableValuedSqlFunctionExpression(new SqlFunctionExpression(_sqlFunctionExpression.FunctionName, Type, _sqlFunctionExpression.Schema, newArguments), QuerySource, Alias)
                 : this;
         }