dotnet / dotnet/efcore

Clean up nullable value type handling in RelationalProjectionBindingExpresisonVisitor

Open
#36,572 1 comment 0 reactions 0 assignees View on GitHub
area-query
Dominant language
C#
Stars
14.8k
Forks
3.4k
PR merge metrics
PR metrics pending

Description

In RelationalProjectionBindingExpressionVisitor, we currently add a convert node up to nullable value types for everything we project; then, at the end we call MatchTypes, which adds a convert node back down to the non-nullable value type if the Select actually projects a non-nullable type.

That's a code smell - we should ideally only be changing the projected type to nullable if the Select actually projects it (and not convert to it and back for when it doesn't). This would also allow us to simply pass in the Select's return type (the nullable value type), rather than generating the nullable value type with MakeGenericType (bad for NativeAOT/trimming).

Ideally, we'd also not use wrapping convert nodes here, but modify the type of the shaper expression we return; this is already done for StructuralTypeShaperExpression (structural type projections), but not for scalars (i.e. we should be able to simply change the type of the ProjectionBindingExpression in a similar way).

/cc @cincuranet

Consider cleaning up the Cosmos projection binding visitor as well - it's far behind the relational counterpart.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.