Query: Add support for instance based SqlFunction #10370
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smitpatel
commented
Nov 22, 2017
| @@ -1348,6 +1363,7 @@ public virtual Expression VisitSqlFunction(SqlFunctionExpression sqlFunctionExpr | |||
| /// <param name="functionName">The function name</param> | |||
| /// <param name="arguments">The function arguments</param> | |||
| /// <param name="schema">The function schema</param> | |||
| [Obsolete("Override VisitSqlFunction method instead.")] | |||
There was a problem hiding this comment.
I added obsolete to avoid breaks. (3 of them in this file)
It was dead and obscure coding style. Only providers would be overriding them.
Break or obsolete?
smitpatel
force-pushed
the
smit/functions
branch
from
f3a926b to
e11f279
Compare
anpete
reviewed
Nov 27, 2017
| @@ -16,7 +16,7 @@ namespace Microsoft.EntityFrameworkCore.Query.Expressions | |||
| /// <summary> | |||
| /// Represents a SQL function call expression. | |||
| /// </summary> | |||
| [DebuggerDisplay("{this.FunctionName}({string.Join(\", \", this.Arguments)})")] | |||
| [DebuggerDisplay("{this.Instance}.{this.FunctionName}({string.Join(\", \", this.Arguments)})")] | |||
There was a problem hiding this comment.
Make helper method and call so you can handle null Instance case?
|
We ideally want some test coverage for this. The problem is that we need a "non-static" function to call on the server. One thing we could do is create a test method call translator that binds to some non-existent store function. We could then swallow the execution error but at least be able to verify the SQL. cc @ajcvickers |
smitpatel
force-pushed
the
smit/functions
branch
from
e11f279 to
3c57461
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #10109