Default a value-type non-entity left-join projection to default(T) on no-match#38555
Conversation
There was a problem hiding this comment.
Pull request overview
Extends the relational outer-join nullability-marker mechanism so that whole-object projections of MemberInit-bound value types (mutable structs/record structs) from the nullable side of a left join default to default(T) on no-match rows, aligning EF Core behavior with LINQ-to-Objects DefaultIfEmpty semantics and avoiding Nullable object must have a value during materialization.
Changes:
- Enable recording/injection of the nullability marker for value-type
New/MemberInitinner shapers on the nullable side of outer joins. - Update projection gating to return
default(T)(zeroed struct) for value-type whole-object projections on no-match rows. - Add/flip relational characterization tests to assert correct struct and
Nullable<T>-wrapped struct behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/EFCore.Relational.Specification.Tests/Query/AdHocMiscellaneousQueryRelationalTestBase.cs | Adds/updates tests asserting value-type whole-object projections default correctly on no-match and preserves matched-row invariants. |
| src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs | Records the nullability marker for New/MemberInit inner shapers on nullable-side outer joins (now including value types). |
| src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs | Gates whole-object non-entity projections on the marker, returning default(T) for value types on no-match rows. |
| } | ||
|
|
||
| [Fact] | ||
| public virtual async Task Struct_whole_object_GroupJoin_DefaultIfEmpty() |
There was a problem hiding this comment.
Add overrides that assert SQL baseline for these tests. I wonder why the override check test isn't catching this.
There was a problem hiding this comment.
Added the missing SqlServer overrides asserting SQL baselines for the whole-object family tests.
On why the check didn't flag it:
- TestHelpers.AssertAllMethodsOverridden is only run when a test class explicitly calls it, and AdHocMiscellaneousQuerySqlServerTest doesn't
- It only works with [ConditionalFact]/[ConditionalTheory] methods — this base uses plain [Fact] throughout.
Note that using [Fact] can mess with assembly exclusions--unless something has changed, we should use Conditional* everywhere.
There was a problem hiding this comment.
Note that using [Fact] can mess with assembly exclusions--unless something has changed, we should use Conditional* everywhere.
We moved to XUnit V3 and now using a different filtering method. I'll update AssertAllMethodsOverridden
… no-match Extends the dotnet#30915 nullability-marker mechanism (previously reference-type only) to MemberInit-bound struct/record struct whole-object projections from the nullable side of an outer join. Previously the marker was only recorded for reference-type inners, so a no-match row still materialized the struct from all-NULL columns and threw "Nullable object must have a value". Gating to default(T) instead matches LINQ-to-Objects DefaultIfEmpty semantics for a value-type sequence, which never produces a true null for a struct. As a side effect, this also fixes the adjacent Nullable<T>-wrapped-struct case, since it shares the same underlying MemberInit shaper: a no-match row now correctly yields HasValue == true with zeroed fields rather than throwing, since casting a real (if zeroed) struct to Nullable<T> can never itself produce null. Part of the dotnet#22517 follow-up work tracked after dotnet#30915/dotnet#38479.
- Rewrite GateNonEntityOnNullabilityMarker / AddJoin recording comments to match actual behavior (value-type MemberInit gated to zeroed struct; Nullable<T> via inner MemberInit + Convert; ctor-bound value types get an unconsumed marker). - Add SqlServer overrides asserting SQL baselines for the dotnet#30915/dotnet#22517 whole-object family tests that lacked them (translation-failure cases assert empty SQL).
0980064 to
68a26d1
Compare
AndriySvyryd
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
…default gate After rebasing onto main, dotnet#38555 (value-type non-entity left-join defaults to default(T) on no-match) closes the value-type materialization gap that ValueTuple_whole_object_from_nullable_side previously asserted as a throw. Flip it to assert the materialized results ((1,2)/(0,0)/(3,1); the no-match row correctly yields default(ValueTuple) = (0,0)), fill in the SqlServer SQL baseline, and update the dotnet#30915 region comments so ValueTuple/mutable-struct are listed as covered rather than deferred.
Extends the #30915 nullability-marker mechanism (previously reference-type only) to MemberInit-bound struct/record struct whole-object projections from the nullable side of an outer join.
Previously the marker was only recorded for reference-type inners, so a no-match row still materialized the struct from all-NULL columns and threw "Nullable object must have a value". Gating to default(T) instead matches LINQ-to-Objects DefaultIfEmpty semantics for a value-type sequence, which never produces a true null for a struct.
As a side effect, this also fixes the adjacent Nullable-wrapped-struct case, since it shares the same underlying MemberInit shaper: a no-match row now correctly yields HasValue == true with zeroed fields rather than throwing, since casting a real (if zeroed) struct to Nullable can never itself produce null.
Part of the #22517 follow-up work tracked after #30915/#38479.