Consider doing property/navigation binding entirely from RelationalSqlTranslatingExpressionVisitor
- Dominant language
- C#
- Stars
- 14.8k
- Forks
- 3.4k
- PR merge metrics
- PR metrics pending
Description
In our current query pipeline design, before RelationalQueryableMethodTranslatingEV sends a lambda body to translation in RelationalSqlTranslatingEV, it pre-visits the lambda (ExpandSharedTypeEntities, invoked from RemapLambdaBody). This pre-visitation pass "expands" property accesses into owned entities, effectively performing the binding at that point. When the lambda body later reaches SQL translation, the property access has already been dealt with.
It would be more natural to pass the lambda body as-is and do all translation inside RelationalSqlTranslatingEV, including binding of owned navigations etc. Aside from being more understandable, this would also reduce an additional visitation into all lambda bodies. I suspect this is done this way since RelationalSqlTranslatingEV currently doesn't know about the SelectExpression being processed; it only receives the lambda body as input.
Regular properties, on the other hand, are bound inside RelationalSqlTranslatingEV as expected. However, there as well, since the SelectExpression isn't available, it's not possible to produce the ColumnExpression at that point. Instead, EntityProjectionMapping contains a dictionary mapping all the entity's properties to ColumnExpressions; this dictionary is created in SelectExpression wherever an EntityProjectionMapping is instantiated, and is used from RelationalSqlTranslatingEV when binding a property.
The fix here would be for ProjectionBindingExpression to have a mapping between relational ITableBase and the TableReferenceExpressions of its SelectExpression. This would allow it to bind properties (and navigations) upon demand - given an IProperty, it would simply find the right ITable for it, get the TableReferenceExpression for that for the current SelectExpression, and construct a ColumnExpression over that.
Doing this would remove the property and owned navigation maps from EntityProjectionExpression, and hopefully simplify a lot of the logic for maintaining these.
/cc @maumar
Contributor guide
Assessment
This issue has not been assessed yet.