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

DefaultQuerySqlGenerator.VisitPropertyParameter bug allows duplicate parameters #14645

Closed
MaxG117 opened this issue Feb 8, 2019 · 2 comments · Fixed by #16469
Closed

DefaultQuerySqlGenerator.VisitPropertyParameter bug allows duplicate parameters #14645

MaxG117 opened this issue Feb 8, 2019 · 2 comments · Fixed by #16469
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@MaxG117
Copy link

MaxG117 commented Feb 8, 2019

DefaultQuerySqlGenerator.VisitPropertyParameter attempts to avoid duplicates by comparing InvariantName with propertyParameterExpression.PropertyParameterName, but adds new parameters using propertyParameterExpression.Name for InvariantName:

            if (_relationalCommandBuilder.ParameterBuilder.Parameters
                .All(p => p.InvariantName != propertyParameterExpression.PropertyParameterName))
            {
                _relationalCommandBuilder.AddPropertyParameter(
                    propertyParameterExpression.Name,
            ...

You can reproduce by making a slight modification to the Where clause of the SimpleQueryTestBase.Where_poco_closure() test case:

            var customer = new Customer
            {
                CustomerID = "ALFKI",
            };

            await AssertQuery<Customer>(
                isAsync,
                cs => cs
                    .Where(c => c.Equals(customer) && c.Equals(customer))
                    .Select(c => c.CustomerID));

Even though the modification is pointless, it's valid syntax and it generates valid CommandText. But when executed against SqlServer EFCore provider, this test throws the following SqlException:

System.Data.SqlClient.SqlException : The variable name '@__customer_0_CustomerID' has already been declared. Variable names must be unique within a query batch or stored procedure.)
@MaxG117
Copy link
Author

MaxG117 commented Feb 8, 2019

I think there's a bigger problem here in that the InvariantName is used as a Key to the parameterValues dictionary in RelationalCommand.CreateCommand, which uses the Name, not the PropertyParameterName. The LINQ to check for duplicates would have to look at the Name, but the Name is not part of the IRelationalParameter Interface.

@ajcvickers ajcvickers added this to the 3.0.0 milestone Feb 11, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, Backlog Jun 28, 2019
roji added a commit to roji/efcore that referenced this issue Jul 9, 2019
Entity equality introduces member access expressions on what may be a
parameter or a constant. Identify these cases and generate a new
parameter (for access of a parameter) or evaluate the constant.

Fixes dotnet#15855
Fixes dotnet#14645
Fixes dotnet#14644
roji added a commit that referenced this issue Jul 9, 2019
Entity equality introduces member access expressions on what may be a
parameter or a constant. Identify these cases and generate a new
parameter (for access of a parameter) or evaluate the constant.

Fixes #15855
Fixes #14645
Fixes #14644
roji added a commit that referenced this issue Jul 9, 2019
Entity equality introduces member access expressions on what may be a
parameter or a constant. Identify these cases and generate a new
parameter (for access of a parameter) or evaluate the constant.

Fixes #15855
Fixes #14645
Fixes #14644
roji added a commit that referenced this issue Jul 9, 2019
Entity equality introduces member access expressions on what may be a
parameter or a constant. Identify these cases and generate a new
parameter (for access of a parameter) or evaluate the constant.

Fixes #15855
Fixes #14645
Fixes #14644
@roji roji closed this as completed in b5cee2d Jul 9, 2019
@roji roji modified the milestones: Backlog, 3.0.0 Jul 9, 2019
@roji
Copy link
Member

roji commented Jul 9, 2019

Note: moved this to the 3.0.0 milestone.

@smitpatel smitpatel removed their assignment Jul 9, 2019
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jul 25, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview8 Jul 29, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview8, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants