-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Query: Fix #1664 - Tagged or named queries #12069
Conversation
anpete
commented
May 18, 2018
- Adds WithTag query operator. From the doc comments: "Adds a tag to the collection of tags associated with an EF LINQ query. Tags are query annotations that can provide contextual tracing information at different points in the query pipeline." The first application of tags, included in this PR, is to cause a diagnostic header comment to be prefixed to generated SQL queries. E.g.
- Also fixes a bug uncovered in the new parameterization parameter caching - NotParameterized parameters are removed from the set of parameters values, but we can't do that if the parameter is being used in more than one place. Fix is to add simple ref counting to the parameter cache.
{ | ||
if (_tags == null) | ||
{ | ||
_tags = _queryCompilationContext?.QueryAnnotations != null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conflicts with #11601
I know “Foo” and “Bar” are overdone, but why would you name or tag your example query “Laurel”? |
@tuespetre It's actually "Yanni". |
@@ -34,8 +34,10 @@ public class ParameterExtractingExpressionVisitor : ExpressionVisitor | |||
private readonly bool _parameterize; | |||
private readonly bool _generateContextAccessors; | |||
|
|||
private readonly Dictionary<Expression, (ParameterExpression Parameter, int RefCount)> _parameterCache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have regression test for this bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, exposed by new test Duplicate_tags
. The same thing would probably happen for duplicate sql in call to FromSql.
- Adds WithTag query operator. From the doc comments: "Adds a tag to the collection of tags associated with an EF LINQ query. Tags are query annotations that can provide contextual tracing information at different points in the query pipeline." The first application of tags, included in this PR, is to cause a diagnostic header comment to be prefixed to generated SQL queries. E.g. ```sql -- EFCore: (#Yanni) SELECT TOP(1) [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] FROM [Customers] AS [c] ORDER BY [c].[CustomerID] ``` - Also fixes a bug uncovered in the new parameterization parameter caching - NotParameterized parameters are removed from the set of parameters values, but we can't do that if the parameter is being used in more than one place. Fix is to add simple ref counting to the parameter cache.