Our funcletizer identifies captured variables in the incoming query tree (member accesses over compiler-generated closure types), and converts these to ParameterExpressions which represent external query parameters; these are later translated to e.g. SqlParameter. These ParameterExpressions have a very different meaning than the incoming compiler-generated ParameterExpression, which represent lambda parameters which never exist in the final translation of the query (they are replaced during translation with the thing they represent).
This design has the following issues:
The query pipeline needs to handle these two types of ParameterExpressions in different ways (query parameters are translated to SqlParameter, lambda parameters aren't). To distinguish between the two, we prefix query parameters with __, and use string pattern matching. This is a good solution, and probably means that if someone has a lambda parameter starting with __, that would fail in various ways. We later also need to remove the prefix, since the actual SqlParameter should have it.
We're starting to need to know various "extra" information about query parameters:
When precompiling queries, static analysis of the query provides us with NRT nullability information for reference types; that is, we're able to know whether a ParameterExpression of type string can actually contain null or not. Since this information can't be transmitted via ParameterExpression, we have QueryCompilationContext.NonNullableReferenceTypeParameters, to record which parameters are non-nullable.
Similarly, in 9.0 we changed the behavior of EF.Parameter() to record that a query node is not to be constantized, even if it otherwise would (#34345). To implement this, we again introduced QueryCompilationContext.ParametersToNotConstantize
This is generally not a great design: the extra information is kept in external data structures rather than in the node itself (and so the structures need to be passed around). More importantly, the data structures refer to the ParameterExpressions by their name, to prevent a situation where some visitor replaces a ParameterExpression for any reason, and we lose referential integrity. Referencing by name also isn't ideal (at least in theory names may be changed as well). @cincuranet we discussed this while designing #34345 and friends).
The solution here seems quite simple: introduce a new QueryParameterExpression which is completely unrelated to ParameterExpression (extension node):
As we'd own this type, we can add whatever metadata we want (non-nullable, not-to-be-constantized...). No more need for external data structures referencing ParameterExpressions in some way.
We'd have a clear node type distinction between query parameters and lambda parameters. This is a good idea, since we always want to do different things for the two node types.
We already have lots of other EF extension types in the pretranslated query tree (e.g. all the query roots), so there would be nothing new here really.
Note: this would be a breaking change to (non-relational) providers as they'd need to translate the new node to whatever their query parameter is (SqlExpression for relational).
Our funcletizer identifies captured variables in the incoming query tree (member accesses over compiler-generated closure types), and converts these to ParameterExpressions which represent external query parameters; these are later translated to e.g. SqlParameter. These ParameterExpressions have a very different meaning than the incoming compiler-generated ParameterExpression, which represent lambda parameters which never exist in the final translation of the query (they are replaced during translation with the thing they represent).
This design has the following issues:
__, and use string pattern matching. This is a good solution, and probably means that if someone has a lambda parameter starting with__, that would fail in various ways. We later also need to remove the prefix, since the actual SqlParameter should have it.The solution here seems quite simple: introduce a new QueryParameterExpression which is completely unrelated to ParameterExpression (extension node):
Note: this would be a breaking change to (non-relational) providers as they'd need to translate the new node to whatever their query parameter is (SqlExpression for relational).