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

Adding a new EF.Functions function with no object references #12527

Closed
MikeDempseyFL opened this issue Jul 2, 2018 · 12 comments
Closed

Adding a new EF.Functions function with no object references #12527

MikeDempseyFL opened this issue Jul 2, 2018 · 12 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@MikeDempseyFL
Copy link

I am trying to write additional EF.Functions that must be executed on the server.

This works well if at least one of the function arguments is a reference to a database column. (via the entity model)

If all arguments are constants, or if there are no arguments, it results in an exception.
This exception occurs without our custom IMethodCallTranslator code getting called.
Linq (?) apparently decides that if there are no object references then it can process the expression locally ... which of course it can not.
The second exception message, below, is from the exception we throw when you try to evaluate one of our extension functions locally.
The stack trace seems to indicate that the problem occurs when initially building the Linq expression tree - before the 'Relational' layer gets a chance to look at it.

Exception:
System.InvalidOperationException : An exception was thrown while attempting to evaluate a LINQ query parameter expression.
----> System.InvalidOperationException : Teradata Database specific functions can not be evaluated on the client.

Stack trace:
at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ParameterExtractingExpressionVisitor.Evaluate(Expression expression, String& parameterName)
at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ParameterExtractingExpressionVisitor.TryExtractParameter(Expression expression)
at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ParameterExtractingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
at System.Linq.Expressions.ExpressionVisitor.Visit(ReadOnlyCollection1 nodes) at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ParameterExtractingExpressionVisitor.VisitNew(NewExpression node) at System.Linq.Expressions.NewExpression.Accept(ExpressionVisitor visitor) at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node) at System.Linq.Expressions.ExpressionVisitor.VisitLambda[T](Expression1 node)
at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ParameterExtractingExpressionVisitor.VisitLambda[T](Expression1 node) at System.Linq.Expressions.Expression1.Accept(ExpressionVisitor visitor)
at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
at System.Linq.Expressions.ExpressionVisitor.VisitUnary(UnaryExpression node)
at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ParameterExtractingExpressionVisitor.VisitUnary(UnaryExpression unaryExpression)
at System.Linq.Expressions.UnaryExpression.Accept(ExpressionVisitor visitor)
at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
at System.Dynamic.Utils.ExpressionVisitorUtils.VisitArguments(ExpressionVisitor visitor, IArgumentProvider nodes)
at System.Linq.Expressions.ExpressionVisitor.VisitMethodCall(MethodCallExpression node)
at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ParameterExtractingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ParameterExtractingExpressionVisitor.ExtractParameters(Expression expression)
at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
at Remotion.Linq.QueryableBase1.GetEnumerator() at System.Collections.Generic.List1.AddEnumerable(IEnumerable1 enumerable) at System.Linq.Enumerable.ToList[TSource](IEnumerable1 source)
at Teradata.EntityFrameworkCore.Tests.TeradataQueryTeradataFunctionTests.NumericTests() in C:\AppSourceCurrent\TdNetDp\Tests\EntityFramework\EFCore.Teradata.Tests\TeradataQueryTeradataFunctionTests.cs:line 142
--InvalidOperationException
at Teradata.EntityFrameworkCore.TeradataDbFunctionsExtensions.Random(DbFunctions _, Int32 lowerBound, Int32 upperBound) in C:\AppSourceCurrent\TdNetDp\src\TdDotNetDataProvider\EFCore.Teradata\Extensions\TeradataDbFunctionsExtensions.cs:line 46
at lambda_method(Closure )
at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ParameterExtractingExpressionVisitor.Evaluate(Expression expression, String& parameterName)

Steps to reproduce

Can not be reproduced without an Entity Provider that implements EF.Functions of this type.
Example 1
The function RANDOM(1, 256) should generate a random number between 1 and 256 for each row in the result set. (This random number will generally be used in an expression, but may sometimes need to be returned in the result set.)
Example 2
The function CurrentUser() should return the user name of the user that connected the database session.

Further technical details

EF Core version: 2.1
Database Provider: Teradata.EntityFrameworkCore
Operating system: Windows 10
IDE: Visual Studio 2017 15.7.2

@pmiddleton
Copy link
Contributor

Have you registered your IMethodCallTranslator in your class which derives from RelationalCompositeMethodCallTranslator?

@MikeDempseyFL
Copy link
Author

MikeDempseyFL commented Jul 3, 2018 via email

@pmiddleton
Copy link
Contributor

I did some testing and this is a bug in EF.

Relinq is detecting that all of the parameters to this call are constants during the EvaluatableTreeFindingExpressionVisitor.Analyze call in ParameterExtractingExpressionVisitor.ExtractParameters.

This is in turn firing RelationalEvaluatableExpressionFilter.IsEvaluatableMethodCall. In that call we don't check any of the methods registered via the ICompositeMethodCallTranslator. Therefore the code compiles and executes the method lambda to evaluate it before translation.

RelationalEvaluatableExpressionFilter.IsEvaluatableMethodCall needs to a way to ask RelationalCompositeMethodCallTranslator if a given method call is translatable.

cc @smitpatel @ajcvickers

I might have some time tomorrow to look into a fix.

@ajcvickers
Copy link
Contributor

@MikeDempseyFL @pmiddleton This is currently a by-design behavior, but it is something we have discussed changing. We will discuss again in triage.
/cc @divega

@pmiddleton
Copy link
Contributor

@ajcvickers

Ok - I'm going to hold off on looking at it then. Let me know what comes out of triage.

@reberinformatik
Copy link

This seems to break also #11484

@smitpatel
Copy link
Contributor

@MikeDempseyFL - In the essence, you need to register "Functions" which are not server correlated as "non-client evaluatable" separately by registering an implementation of IEvaluatableExpressionFilter deriving from appropriate class.

@pmiddleton - MethodCallTranslator has nothing to do with EvaluatableExpressionFilter and they should not interact with each other.

@MikeDempseyFL
Copy link
Author

Thanks. I implemented an IEvaluatableExpressionFilter class and returned false for the 4 functions that were causing the issue and they are now working correctly.

@smitpatel smitpatel added the closed-no-further-action The issue is closed and no further action is planned. label Jul 6, 2018
@pmiddleton
Copy link
Contributor

@smitpatel - If we want a solution which can handle all cases where the methods parameters are all independent of the rest of the query then either RelationalEvaluatableExpressionFilter or EvaluatableExpressionFilter needs access to the list of provider registered methods stored inside of ICompositeMethodCallTranslator.

Right now that data is not accessible in the form we need. The registered methods are buried inside of numerous IMethodCallTranslator implementations which ICompositeMethodCallTranslator wraps.

Ideally there would be a central repository of mapped methods which could then be passed into the EvaluatableExpressionFilter.

Having that map method repository would then also clean up query translation a bit. Right now when a method needs translation, for use in the back end, there is a O(N) walk over all translators asking each one to try to map the MethodCallExpression. Having a lookup table would turn this into a nice O(1) lookup.

@smitpatel
Copy link
Contributor

There are 3 different aspects being combined into one here.
The query pipeline (pre-compilation phase), want to funcletize as much as possible. i.e. anything independent of range variable will get evaluated to a parameter. The only purpose of adding implementation of IEvaluatableExpressionFilter was to block evaluation of non-deterministic function which would defer in value on server/client side. I am bit skeptical of why do we need Relational one to block evaluation of DbFunction since they should mostly be dependent on range variable (read part 2 too).

Since we have a mechanism to block evaluation, we can abuse the system to block evaluation of functions which we know will throw exception on client side. i.e. methods which have methodcall translators but no client side implementation. I believe relational level did exactly this for DbFunctions. Same is guidance for above (for now).

Having MethodCallTranslator is independent of evaluating it on client side if possible. Suppose user has a constant datetime followed by bunch of Add* methods, we can still generate single parameter rather than a large SQL which would evaluate to same value. Since except for UDFs & EF.Functions, everything else in BCL is supposed to have client side implementation anyway. Hence EvaluatableExpressionFilter should not depend on MethodCallTranslators at all. We abuse the system to make sure certain functions are never client evaluated (or may be design a different system for that - subjective to value).

As for O(N) vs O(1), if there is a way to make it more efficient, then we should just do it for MethodCallTranslator. It is still useful in the absence of whatever is being discussed in this issue. Feel free to file a new issue with slightly more detailed idea of yours. Its a good perf improvement.

@pmiddleton
Copy link
Contributor

In regards to DbFunctions while it is true that they should mostly be dependent on range variables it is not true 100% of the time.

I myself have a key function in one of my big applications which takes all client provided integers, but can not be evaluated client side because it needs to query a number of tables.

There are also a number of built in functions in Sql Server which if mapped could fall into this category as well depending on how they are used (HAS_DBACCESS, IS_ROLEMEMEBER, USER_ID, etc).

@smitpatel
Copy link
Contributor

@pmiddleton - That is useful information. Hence abuse of system perhaps. A subcategory of DbFunctions & EF.Functions methods could be independent of range variable yet needs to be server evaluated. In which case, we block it on client side. Though not all methodcalltranslators are about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

5 participants