Query: chaining multiple collection navigations followed by FirstOrDefault returns incorrect results #15798
Comments
|
@divega to have a bit of a think. |
|
@maumar I found a bit interesting that the SQL translation produced by EF Core 2.2 for the second query does not include terms to compensate for three value logic and produces the correct result: SELECT (
SELECT TOP(1) [od].[Name]
FROM [OrderDetails] AS [od]
WHERE [od].[OrderId] = (
SELECT TOP(1) [o].[Id]
FROM [Orders] AS [o]
WHERE [o].[CustomerId] = [c].[Id]
ORDER BY [o].[Id]
)
ORDER BY [od].[Id]
)
FROM [Customers] AS [c]While the 3.0 translation is: SELECT (
SELECT TOP(1) [o].[Name]
FROM [OrderDetails] AS [o]
WHERE (([o].[OrderId] = (
SELECT TOP(1) [o0].[Id]
FROM [Orders] AS [o0]
WHERE [o0].[CustomerId] = [c].[Id]
ORDER BY [o0].[Id])) AND ([o].[OrderId] IS NOT NULL AND (
SELECT TOP(1) [o0].[Id]
FROM [Orders] AS [o0]
WHERE [o0].[CustomerId] = [c].[Id]
ORDER BY [o0].[Id]) IS NOT NULL)) OR ([o].[OrderId] IS NULL AND (
SELECT TOP(1) [o0].[Id]
FROM [Orders] AS [o0]
WHERE [o0].[CustomerId] = [c].[Id]
ORDER BY [o0].[Id]) IS NULL)
ORDER BY [o].[Id])
FROM [Customers] AS [c]Although this might be just a fortunate coincidence (and the reason for it might simply be that we didn't fully implement null semantics ), I agree with you that in this case the null semantics expansion doesn't apply. At first glance, it seems that this is true in general when we compare FK to PK properties. In other words, when we traverse relationships (e.g. also for any joins). I cannot think of a better criteria we can use to decide when this behavior applies. Incidentally, even in LINQ to Objects, Join() does not yield rows on which both sides of the equals are null. This also made me wonder what the translation should be if Order had a composite key, but currently (with preview6) the query is broken (exception at the end). It seems to me that it should be possible to produce a translation that would work with composite keys using joins and window functions (again!) to return the first rows of orders and order details. Is there something simpler? Here is the exception message you get in preview 6 with a composite key on Order: |
|
In 2.2 we deliberately did not apply null semantics in this case. When constructing a collection navigation we created dedicated expression representing correlation predicate, and when applying null semantics we would not apply null semantics for it (since it was custom expression, it was super easy to detect the case). That was my original plan but @smitpatel was adamant against it. Wrt composite keys, I was thinking about something like this (third example): #15260 but I'm not sure if the approach can be generalized. |
Ok, I believe the solution would look like this, but I think we need to think trough and come up with a rational rule for when the behavior applies. E.g. is it really only for collection navigations? What about regular joins and references? I think it is all of the above because you simply never use null values to establish a relationship.
Yep, that looks like it should work. |
Incorrect. We did not do that at all. It is in where predicate and we did not fully expand it since we assumed null is fall so we don't need it. The only case we did not apply null semantics in 2.x was single key join. Even for composite key we just went along and applied null semantics. (Though we may have treated it like predicate) |
|
@smitpatel we created NullSafeEqualExpression (bad name) when expanding collection navigation and during translation we wrapped it around NullCompensatedExpression so that null semantics doesnt peek in. It does seem wrong for complex cases where scalar subquery is part of the correlation predicate and it has some stuff inside that would need null semantics. But for cases of simple navigation access it was fine. |
|
As of now in new pipeline, I don't see NullSafeEqualExpression while translating to SQL. The thing which I argued against was to generate null check for outer key rather than inner key. |
|
I discussed this further with @smitpatel. Here are some conclusions:
|
|
verified this issue has been fixed in 3.0 |
maumar
added
closed-fixed
query:
we translate this to:
Now, because we apply C# semantics to the comparison, when od.OrderId is NULL and the orders subquery is null, we match this to true and return the associated order detail. This is not expected - we should filter out elements where null == null (i.e. simulate database null semantics for this specific case).
The text was updated successfully, but these errors were encountered: