Materialize a left-joined non-entity projection as null on no-match (#30915)#38479
Open
ajcvickers wants to merge 11 commits into
Open
Materialize a left-joined non-entity projection as null on no-match (#30915)#38479ajcvickers wants to merge 11 commits into
ajcvickers wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a long-standing relational materialization bug where a LEFT JOIN/DefaultIfEmpty() whose nullable side projects a non-entity object (anonymous type/DTO/aggregate projection) could throw InvalidOperationException: Nullable object must have a value on no-match rows, by ensuring the whole projected object becomes null instead of being constructed from all-NULL columns.
Changes:
- Injects and tracks a synthetic “marker” projection on the nullable side of outer joins inside
SelectExpression, and uses it to null-gate whole-object non-entity projections during projection binding. - Adds extensive relational characterization coverage for the #30915 shape matrix, with provider-specific SQL baselines for SQLite and SQL Server.
- Adds provider-specific overrides for the correlated
OUTER APPLYcase on SQL Server (provider divergence vs. SQLite).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/EFCore.SqlServer.FunctionalTests/Query/AdHocMiscellaneousQuerySqlServerTest.cs | Adds #30915 seeding and SQL Server-specific SQL baselines/overrides (including OUTER APPLY case). |
| test/EFCore.Sqlite.FunctionalTests/Query/AdHocMiscellaneousQuerySqliteTest.cs | Adds #30915 seeding and SQLite SQL baselines for the fixed shapes. |
| test/EFCore.Relational.Specification.Tests/Query/AdHocMiscellaneousQueryRelationalTestBase.cs | Adds the main characterization test matrix for left-joined non-entity whole-object materialization and deferred follow-ups. |
| src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs | Implements marker injection/aliasing and marker bookkeeping on outer joins to enable null-gating later. |
| src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs | Applies marker-based gating when projecting whole non-entity objects, while avoiding interference with TransparentIdentifier intermediate projections. |
Contributor
Author
|
@AndriySvyryd I did a lot of analysis of similar query shapes and added tests to try to reduce the risk of regressions here. There are still a lot of related cases that don't work--there is a checklist in the PR description. @roji I know it's not your job, but if you have time it would be good to get your eyes on this. |
… projections (dotnet#30915 baseline)
…net#30915) A LEFT JOIN (GroupJoin+DefaultIfEmpty or the LeftJoin operator) whose inner side is a subquery projecting a non-entity (anonymous type, DTO, or a GroupBy-with-aggregate projection) threw 'Nullable object must have a value' when the whole projected object was selected and the join produced a no-match (all-NULL) row: EF constructed the object instead of yielding null. Entities already handle this via key-column null checks. For non-entity projections, inject a synthesized marker column (1, aliased 'marker') into the inner subquery on the nullable side so the LEFT JOIN nulls it on no-match, record (inner-shaper-node -> marker binding) on the SelectExpression, and gate whole-object materialization in RelationalProjectionBindingExpressionVisitor so it yields null when the marker is null. Member access is unaffected (it folds to a column before reaching the gate), and value-type / transparent-identifier projections are left untouched. Covers direct whole-object projection of a left-joined non-entity. Does not yet cover: a subsequent GroupBy/second-join, a non-pushed-down inner, value-type structs, or server-side object == null (tracked as follow-ups).
…rojection, alias collision, boundary cases
…30915) When a user projection has a member named 'marker' beside the projected nullable object, the synthesized marker uniquifies at every subquery level but leaves a cosmetic duplicate output alias at the outermost SELECT (which never uniquifies, for any projection - EF binds by ordinal). Legal SQL; self-heals under composition. Comment points to the pinning tests so this isn't re-opened as a bug.
TryGetNonEntityNullabilityMarker and RemapNonEntityNullabilityMarker are query-translation operations on the public SelectExpression type, in the same category as the existing pubternal ApplyDefaultIfEmpty / MakeProjectionNullable. Match that house style: public + [EntityFrameworkInternal] + the standard internal-API doc disclaimer, rather than real 'internal'. (NullabilityMarkerMemberInfo stays internal: a constant-like shared detail, cf. internal const DiscriminatorColumnAlias.)
…-type/Nullable<T> conflation
…ner, DISTINCT marker, Nullable<T>, ValueTuple, deep member access)
ajcvickers
force-pushed
the
fix/30915-defaultifempty-nonentity-null
branch
from
66877b0 to
f4e208e
Compare
…arker column Left_join_on_entity_with_owned_navigations_complex projects an anonymous type after grouping.DefaultIfEmpty(), so the dotnet#30915 nullability marker (1 AS [marker]) is now legitimately emitted in the DISTINCT subquery. Data assertions already pass on both providers; only the SQL Server baselines needed updating (SQLite does not baseline this query's SQL).
| // Category A: whole non-entity object projected from the nullable side | ||
| // --------------------------------------------------------------------------------------- | ||
|
|
||
| [Fact] // 1 |
Member
There was a problem hiding this comment.
Remove numbering comments.
Use regions to group tests instead of just comments
| private static bool IsTransparentIdentifierProjection(Expression expression) | ||
| => expression is NewExpression newExpression && IsTransparentIdentifierType(newExpression.Type); | ||
|
|
||
| private static bool IsTransparentIdentifierType(Type type) |
Member
There was a problem hiding this comment.
Move this to TransparentIdentifierFactory, there's probably more code that could use this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A
LEFT JOIN— viaGroupJoin+DefaultIfEmpty()or theLeftJoinoperator — whose inner side is a subquery projecting a non-entity (an anonymous type, a DTO, or aGroupBy-with-aggregate projection), when the whole projected object is selected, threwInvalidOperationException: Nullable object must have a valueon a no-match (all-NULL) row. EF constructed the object from all-NULLcolumns instead of yieldingnull.Entities already get this right via key-column null checks; this does the equivalent for non-entity projections.
Addresses #30915 and its duplicate #38055; partially implements the umbrella #22517. This is not a complete fix of that family — several related shapes remain deferred (see the checklist below), each pinned by a characterization test so a future change is detected.
Approach
Mirror the entity path without a structural wrapper (a wrapper around the inner shaper breaks
NewExpression/MemberInitExpressionmember-folding in coreReplacingExpressionVisitorandRelationalSqlTranslatingExpressionVisitor, which would regress member-only consumers):1 AS marker, aSqlFragmentExpression) into the inner subquery on the nullable side, before pushdown, so theLEFT JOINnulls it on a no-match row.(inner-shaper-node → marker binding)on theSelectExpression, re-keyed through the transparent-identifier projection-binding pass.RelationalProjectionBindingExpressionVisitor:marker == null ? null : new { ... }. Member access folds to a column before reaching the gate, so member-only consumers are untouched.The marker is injected only for reference-type
New/MemberInitinners on the nullable side, and is pruned when unconsumed — so existing entity and member-only queries are byte-identical (verified below).Testing
Built as a golden-master baseline to prove the change is additive: the characterization tests were committed first on
main(asserting current behavior — correct results where it works, asserted-throw / translation-fail where it doesn't), then the fix, then only the tests whose behavior actually changed were flipped to assert the corrected behavior.Distinct,Take,GroupBy-after, second-join-after,Union); server-side null checks; alias collision with a user member namedmarker; and the matched-row-with-null-data invariant — green on SQLite and SQL Server.Follow-up work (deferred; each already pinned by a characterization test)
All are sub-cases of the umbrella #22517 unless noted.
GroupByapplied after the left join, then whole-object projection (shaper rebuilt viaApplyGrouping). Repro: closed dup DefaultIfEmpty() unsupported in subquery when main query has group by? #28119.LeftJoinoperator — marker is inlined into the combinedSELECTand never becomesNULL. (no dedicated issue)struct/record structwhole-object projection (members not defaulted; materialization throws).record struct/ValueTuplewhole-object projection (fails at translation — a distinct path). Overlaps GroupJoin throws when applied on an anonymous return type output of another Linq #28248.Union/ set-operation of two left-join non-entity projections (set-operation translation failure). (no dedicated issue)obj == null/obj != nullon a left-joined non-entity, inWhere/OrderBy(translation failure). A complete Result of left joining a nullable nominal type should be null (i.e. no instance) and not an instance with all null property values #22517 (true null-tracking) would likely enable this. (no dedicated issue)RightJoinwhere the outer (non-entity) side is the nullable one — the marker is injected for the inner side only, so this still throws.Nullable<T>whole-object projection (latent and currently unreachable; the gate skips onIsValueType, butdefault(T?)isnull).InMemoryQueryExpression.AddJoin(this PR is relational-only). Related InMemory case EFCore.InMemory: "System.InvalidOperationException : Nullable object must have a value." when doing second left join via object #23425 was closedNOT_PLANNED.Confirmed working (not follow-ups), from the added coverage
SelectMany+DefaultIfEmptywhole-object works on providers that supportOUTER APPLY(e.g. SQL Server); it fails on SQLite only because SQLite has noAPPLY(a provider limitation, not this bug).markerdoes not collide with the synthesized marker column — aliases uniquify (marker/marker0); pinned by a test, and the cosmetic outermost-SELECTduplicate (read by ordinal) self-heals under composition.nullstays non-null, distinct from a no-match — the invariant that justifies the marker design over member-coalescing; pinned by a test.Issue housekeeping
#30915, #38055, #22517 and #28248 are the same problem family but currently disconnected — worth cross-linking.