Skip to content

Conversation

@roji
Copy link
Member

@roji roji commented Nov 25, 2024

  • Remove our query parameter prefix (__), and implement proper uniquification in the funcletizer - only as needed - rather than always adding on a running number. This causes our basic parameter name to change from @__city_0 to @city.
  • While the above - in the funcletizer - takes care of uniquification across regular parameters, it does not do so for FromSql parameters, which may have the same name and therefore conflict. Allowing the funcletizer to know about DbParameters would mean making it provider-specific (it's currently universal and internal).
  • To implement the uniquification, worked logic into FromSqlParameterExpandingExpressionVisitor, which was previously purely about "expanding" FromSql parameters; it now also performs uniquification across all parameters (DbParameter and regular). Renamed the visitor to RelationalParameterProcessor (it's internal).
  • As long as we're at it, also moved the logic that sends two distinct parameters when the same parameter is used twice in the query, with different type mappings; this used to be in QuerySqlGenerator.VisitSqlParameter, moved it into the new RelationalParameterProcessor as well (this way all parameter handling is concentrated in a single visitor, and QuerySqlGenerator is kept as a very thin SQL generator, with minimal logic).
  • Since we may need to change the SqlParameterExpression name as part of uniquification in this phase (i.e. a DbParameter exists earlier in the tree with the same name), added InvariantName to SqlParameter (InvariantName references the parameter value via QueryContext.ParameterValues, Name is the name that the ADO.NET DbParameter gets; only the latter needs to be modified for uniquification).

Closes #35113
Fixes #35196

@roji roji force-pushed the SimplifyParameterNames branch 3 times, most recently from c6e0f64 to 49fb359 Compare November 25, 2024 16:35
@roji roji marked this pull request as ready for review November 25, 2024 16:35
@roji roji requested a review from a team as a code owner November 25, 2024 16:35
@roji roji enabled auto-merge (squash) November 25, 2024 17:05
@roji roji force-pushed the SimplifyParameterNames branch from 49fb359 to 3fc90ba Compare November 25, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DbParameters with same name in FromSql/SqlQueryRaw get incorrectly deduplicated Consider simplifying SQL parameter names (@__city_0 -> @city)

2 participants