Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for complex types projected via optional navigations #31412

Open
Tracked by #31238
roji opened this issue Aug 5, 2023 · 3 comments
Open
Tracked by #31238

Support for complex types projected via optional navigations #31412

roji opened this issue Aug 5, 2023 · 3 comments

Comments

@roji
Copy link
Member

roji commented Aug 5, 2023

We can support projecting complex types just fine, but projecting them via an optional navigation creates a problem:

public virtual Task Project_complex_type_via_optional_navigation(bool async)
    => AssertQuery(
        async,
        ss => ss.Set<CustomerGroup>().Select(cg => cg.OptionalCustomer!.ShippingAddress));

For cases where OptionalCustomer is null, EF currently materializes a non-null ShippingAddress with all-null values; it should be materializing null. The issue is that all we have are the ShippingAddress columns coming back from the database, and no way to know whether the address itself is null or not:

SELECT [c0].[ShippingAddress_AddressLine1], [c0].[ShippingAddress_AddressLine2], [c0].[ShippingAddress_ZipCode], [c0].[ShippingAddress_Country_Code], [c0].[ShippingAddress_Country_FullName]
FROM [CustomerGroup] AS [c]
LEFT JOIN [Customer] AS [c0] ON [c].[OptionalCustomerId] = [c0].[Id]

With e.g. owned entities this doesn't occur since the key columns of the owner entity type are projected as well (they're also the key columns of the owned entity type), and their nullability determines whether the owned is there or not.

The solution here would be to do a similar thing and project a single key property from the containing entity type, and then check that in the materializer to determine whether ShippingAddress should be null or not.

However, as @maumar pointed out, this is probably incompatible with Distinct: if we add the containing key column before the distinct, that effectively prevents DISTINCT from doing its job (since it's unique). We don't think there's a way to solve this in SQL (except maybe in PG where you can do DISTINCT BY 😉), so we should detect this and block it.

@roji
Copy link
Member Author

roji commented Aug 7, 2023

Regarding the interaction of this with Distinct, here's a solution courtesy of @AndriySvyryd. Instead of projecting out an actual ID column of the containing entity type - which would interfere with Distinct - we could project out a bool saying whether that ID column is null or not (effectively whether the entity instance exists or not). This wouldn't interfere with the Distinct any more (since there's no unique ID column, only a bool flag), and we'd use that flag to determine the materialization behavior back at the client (non-null complex type with all-nulls when the flag is true (container exists), null complex type when the flag is false (container does not exist).

/cc @maumar

@ajcvickers ajcvickers added this to the Backlog milestone Sep 6, 2023
@cpa-level-it
Copy link

Hi, is this something that is considered to be included in EF8 ? We are migrating an application from EF6 which heavily uses complex types and a lot of query seems to be broken without it.

In the meantime, if anybody has the same issue you can make it work using a ternary condition on the navigation property key.

Change

var items = query.Select(x => x.SomeOptionalNavigationProperty.AComplexType).ToList();

to

var items = query.Select(x => (int?)x.SomeOptionalNavigationProperty.Id != null ? x.SomeOptionalNavigationProperty.AComplexType : null).ToList();

It probably doesn't cover all the case but it's a start.

@ajcvickers
Copy link
Contributor

This issue is in the Backlog milestone. This means that it is not planned for the next release (EF Core 8.0). We will re-assess the backlog following the this release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources. Make sure to vote (👍) for this issue if it is important to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants