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.

Sign up for GitHub

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

Jump to bottom

Query: Deal with same querycontext parameter having multiple inferred type mapping #19503

Closed
smitpatel opened this issue Jan 7, 2020 · 3 comments · Fixed by #26582
Closed

Query: Deal with same querycontext parameter having multiple inferred type mapping #19503

smitpatel opened this issue Jan 7, 2020 · 3 comments · Fixed by #26582
Assignees
@smitpatel
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release punted-for-6.0 type-bug
Milestone
7.0.0

Comments

@smitpatel
Copy link
Member 

var city = "Settle";
cs.Where(c => c.City == city || c.Address.Contains(city)).ToList();

In above query we have only 1 query context parameter for city but in SQL, city could take different inferred type mapping. We need to determine what should we do with such cases especially if one mapping is unicode and other is not.
It also intersect with the fact that currently we have 1 to 1 mapping between query context parameters and relational parameters which we can relax and use for #17948
@smitpatel smitpatel mentioned this issue Jan 7, 2020
Closed
@ajcvickers ajcvickers added this to the Backlog milestone Jan 10, 2020
@smitpatel smitpatel mentioned this issue Mar 5, 2020
Closed
@smitpatel smitpatel removed this from the Backlog milestone Mar 5, 2020
@ajcvickers ajcvickers added this to the Backlog milestone Mar 6, 2020
@smitpatel smitpatel mentioned this issue Sep 16, 2020
Closed
@ajcvickers ajcvickers modified the milestones: Backlog, 6.0.0 Nov 5, 2020
@smitpatel
Copy link
Member Author

Proposal: Break IRelationalParameter interface and expose TypeMapping.

  • Each relational parameter needs to have type mapping assigned to it for us to generate the DbParameter.
  • Each relational parameter is different from other if it is taking value of a different client side parameter or it has different typemapping then other relational parameter taking same client side value.

@ajcvickers
Copy link
Member

@smitpatel Design meeting?

@smitpatel smitpatel removed this from the 6.0.0 milestone Aug 17, 2021
@ajcvickers ajcvickers added this to the Backlog milestone Aug 20, 2021
@roji roji mentioned this issue Oct 20, 2021
Open
@smitpatel smitpatel mentioned this issue Oct 27, 2021
Closed
@ajcvickers ajcvickers added the priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release label Nov 3, 2021
@ajcvickers ajcvickers modified the milestones: Backlog, 7.0.0 Nov 3, 2021
@smitpatel smitpatel mentioned this issue Nov 8, 2021
Closed
smitpatel added a commit that referenced this issue Nov 8, 2021
@smitpatel
Query: Add multiple parameters if inferred type mapping differs
4b188f0 
When adding parameters from query, we check if the type mapping inside TypeMapped parameter is same, if it is not then we add another parameter with different name in SQL.

Clean up in relational parameter builder code
- Remove Dynamic parameter/ property parameter
- Obsolete TypeMappingSource from CommandBuilder
- Remove obsolete code and combine methods which were doing same work

Resolves #19503
Resolves #22524
Resolves #26472
@smitpatel smitpatel mentioned this issue Nov 8, 2021
Merged
smitpatel added a commit that referenced this issue Nov 9, 2021
@smitpatel
Query: Add multiple parameters if inferred type mapping differs
6d00a22 
When adding parameters from query, we check if the type mapping inside TypeMapped parameter is same, if it is not then we add another parameter with different name in SQL.

Clean up in relational parameter builder code
- Remove obsoleted property parameter
- Obsolete Dynamic parameter
- Obsolete TypeMappingSource from CommandBuilder
- Remove obsolete code and combine methods which were doing same work

Resolves #19503
Resolves #22524
Resolves #26472
@smitpatel smitpatel added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed needs-design labels Nov 9, 2021
@smitpatel
Copy link
Member Author

Didn't need breaking changes like above if we cleaned up the parameters we have atm. The design is in PR.

smitpatel added a commit that referenced this issue Nov 20, 2021
@smitpatel
Query: Add multiple parameters if inferred type mapping differs
04094ce 
When adding parameters from query, we check if the type mapping inside TypeMapped parameter is same, if it is not then we add another parameter with different name in SQL.

Clean up in relational parameter builder code
- Remove obsoleted property parameter
- Obsolete Dynamic parameter
- Obsolete TypeMappingSource from CommandBuilder
- Remove obsolete code and combine methods which were doing same work

Resolves #19503
Resolves #22524
Resolves #26472
smitpatel added a commit that referenced this issue Nov 20, 2021
@smitpatel
Query: Add multiple parameters if inferred type mapping differs
3db1fc8 
When adding parameters from query, we check if the type mapping inside TypeMapped parameter is same, if it is not then we add another parameter with different name in SQL.

Clean up in relational parameter builder code
- Remove Dynamic parameter
- Obsolete TypeMappingSource from CommandBuilder
- Combine methods which were doing same work

Resolves #19503
Resolves #22524
Resolves #26472
smitpatel added a commit that referenced this issue Dec 7, 2021
@smitpatel
Query: Add multiple parameters if inferred type mapping differs
ad963ef 
When adding parameters from query, we check if the type mapping inside TypeMapped parameter is same, if it is not then we add another parameter with different name in SQL.

Clean up in relational parameter builder code
- Remove Dynamic parameter
- Obsolete TypeMappingSource from CommandBuilder
- Combine methods which were doing same work

Resolves #19503
Resolves #22524
Resolves #26472
smitpatel added a commit that referenced this issue Dec 7, 2021
@smitpatel
Query: Add multiple parameters if inferred type mapping differs
2c95c08 
When adding parameters from query, we check if the type mapping inside TypeMapped parameter is same, if it is not then we add another parameter with different name in SQL.

Clean up in relational parameter builder code
- Remove Dynamic parameter
- Obsolete TypeMappingSource from CommandBuilder
- Combine methods which were doing same work

Resolves #19503
Resolves #22524
Resolves #26472
@ghost ghost closed this as completed in #26582 Dec 8, 2021
ghost pushed a commit that referenced this issue Dec 8, 2021
@smitpatel
Query: Add multiple parameters if inferred type mapping differs (#26582)
205dfd1 
When adding parameters from query, we check if the type mapping inside TypeMapped parameter is same, if it is not then we add another parameter with different name in SQL.

Clean up in relational parameter builder code
- Remove Dynamic parameter
- Obsolete TypeMappingSource from CommandBuilder
- Combine methods which were doing same work

Resolves #19503
Resolves #22524
Resolves #26472
@ajcvickers ajcvickers mentioned this issue Jan 6, 2022
Closed
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-preview1 Feb 14, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-preview1, 7.0.0 Nov 5, 2022
@roji roji mentioned this issue Nov 22, 2022
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@smitpatel smitpatel

Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release punted-for-6.0 type-bug
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

Query: Add multiple parameters if inferred type mapping differs dotnet/efcore
3 participants
@ajcvickers @smitpatel @AndriySvyryd