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

Take(1) not being honored in filtered include #24417

Closed
CTGControls opened this issue Mar 16, 2021 · 9 comments
Closed

Take(1) not being honored in filtered include #24417

CTGControls opened this issue Mar 16, 2021 · 9 comments

Comments

@CTGControls
Copy link

The problem

I have made a working example on github, at https://github.com/FlameMetals/SupportExamples. This example is using the In-Memory database but we are using SqlServer database in production. We get the same results in both SqlServer and In-Memory.

In SupportExamples/API/Controllers/orderPartsController.cs on line 70 you will find this code:

FFM.DataAccessModels.App.orderPartsHeader _modelHeader = 
    await _FFM_DbContext.orderPartsHeader
        .Where(m => m.id == id)
            .Include(x => x.orderParts.OrderByDescending(x => x.createdOnDate).Take(1))
                .ThenInclude(x => x.customerParts.customerPartsHeader.customerParts.OrderByDescending(
                    x => x.createdOnDate).Take(1)
                )
        .FirstOrDefaultAsync(m => m.id == id)
;

The problem is the take(1) in .ThenInclude(x => x.customerParts.customerPartsHeader.customerParts.OrderByDescending( x => x.createdOnDate).Take(1) is not being honored.

If you set a breakpoint in SupportExamples/API/Controllers/orderPartsController.cs on line 87 and navigate to "https://localhost:5001/api/orderParts/5AB4CB69-7982-44FA-97CD-03E1D386E5E6" you will see there are two entities (id = bf4349a0-2cba-4646-baa7-0bafeeba9f8c and d13195c6-2d9e-46b0-b633-aa9e074ea15f) in _modelHeader -> orderParts[0] -> customerParts -> customerPartsHeader -> customerParts. The desired result would be the entity with the id of d13195c6-2d9e-46b0-b633-aa9e074ea15f and not entity with the id of bf4349a0-2cba-4646-baa7-0bafeeba9f8c is included in the results.

Below is not the problem just A peculiar observation

If you run the project and navigate to "https://localhost:5001/api/orderParts/5AB4CB69-7982-44FA-97CD-03E1D386E5E6" you will get the results below ->

{
  "ordersHeaderId": "00000000-0000-0000-0000-000000000000",
  "lineNumber": 1,
  "orderParts": [
    {
      "orderPartsHeaderId": "5ab4cb69-7982-44fa-97cd-03e1d386e5e6",
      "orderPartsHeader": null,
      "customerPartsId": "bf4349a0-2cba-4646-baa7-0bafeeba9f8c",
      "customerParts": {
        "customerPartsHeaderId": "596efbff-8c30-4b75-bfdb-ef7e05b5f96d",
        "customerPartsHeader": {
          "customerParts": [
            null,
            {
              "customerPartsHeaderId": "596efbff-8c30-4b75-bfdb-ef7e05b5f96d",
              "customerPartsHeader": null,
              "userfriendlyName": "NEW CUSTOMER PART",
              "name": "Name Changed",
              "description": "New Description",
              "id": "d13195c6-2d9e-46b0-b633-aa9e074ea15f",
              "changeKey": null,
              "createdOnDate": "2019-06-17T09:32:20.426",
              "createdByUserId": "31a1793a-9453-48cd-9909-7c157dae6a8a",
              "createdByIp": ""
            }
          ],
          "id": "596efbff-8c30-4b75-bfdb-ef7e05b5f96d",
          "changeKey": null,
          "isActive": true,
          "isDeleted": false,
          "lastModifiedOnDate": null,
          "lastModifiedByUserId": null,
          "lastModifiedByIp": null,
          "deletedOnDate": null,
          "deletedByUserId": null,
          "deletedByIp": null,
          "approvedRejectedOnDate": null,
          "approvedRejectedByUserId": null,
          "approvedRejectedByIp": ""
        },
        "userfriendlyName": "OLD CUSTOMER PART",
        "name": "name 1",
        "description": "desc 1",
        "id": "bf4349a0-2cba-4646-baa7-0bafeeba9f8c",
        "changeKey": null,
        "createdOnDate": "2018-12-17T09:32:20.426",
        "createdByUserId": "31a1793a-9453-48cd-9909-7c157dae6a8a",
        "createdByIp": ""
      },
      "userfriendlyName": "Order Part 1",
      "orderQty": 100,
      "orderWt": 2.3,
      "id": "579f9060-4b69-4ac6-b1d1-d7988f3f5d26",
      "changeKey": null,
      "createdOnDate": "2018-12-17T09:32:20.426",
      "createdByUserId": "31a1793a-9453-48cd-9909-7c157dae6a8a",
      "createdByIp": ""
    }
  ],
  "id": "5ab4cb69-7982-44fa-97cd-03e1d386e5e6",
  "changeKey": null,
  "isActive": true,
  "isDeleted": false,
  "lastModifiedOnDate": null,
  "lastModifiedByUserId": null,
  "lastModifiedByIp": null,
  "deletedOnDate": null,
  "deletedByUserId": null,
  "deletedByIp": null,
  "approvedRejectedOnDate": null,
  "approvedRejectedByUserId": null,
  "approvedRejectedByIp": ""
}

As you can see we have a null and a customerParts with the id of d13195c6-2d9e-46b0-b633-aa9e074ea15f in _modelHeader -> orderParts[0] -> customerParts -> customerPartsHeader -> customerParts. I am not sure why the data in the breakpoint and what is serialized are not the same. The desired result would be the entity with the id of d13195c6-2d9e-46b0-b633-aa9e074ea15f and not the null is included in the results.

The provider and version information

  • EF Core version:
    • .NET 5.0 in production and are testing .NET 6.0
  • Database provider:
    • Microsoft.EntityFrameworkCore.SqlServer in production but this example is using Microsoft.EntityFrameworkCore.In-Memory
  • Target framework:
    • .NET 5.0 in production and are testing .NET 6.0
  • Operating system:
    • Windows 10 in Dev and Windows Server 2016 in production
  • IDE:
    • Visual Studio 2019 Version 16.10.0 Preview 1.0 and Visual Studio Code
@maumar
Copy link
Contributor

maumar commented Mar 17, 2021

related issue: #23674

@maumar
Copy link
Contributor

maumar commented Mar 17, 2021

Query that gets to EF:

DbSet<orderPartsHeader>()
    .Where(m => (Nullable<Guid>)m.id == __id_0)
    .Include(x => x.orderParts
        .OrderByDescending(x => x.createdOnDate)
        .Take(1))
    .ThenInclude(x => x.customerParts.customerPartsHeader.customerParts
        .OrderByDescending(x => x.createdOnDate)
        .Take(1))
    .FirstOrDefault(m => (Nullable<Guid>)m.id == __id_0)

ThenInclude chain contains a cycle, which we remove and therefore the "filter" part of the include is completely ignored

@ajcvickers
Copy link
Member

Closing as duplicate of #23674

@CTGControls
Copy link
Author

CTGControls commented Mar 24, 2021

I understand the statement that Maurycy Markowski said that "ThenInclude chain contains a cycle". If it is unsupported it is unsupported but the issue was closed as a duplicate of #23674. It's probably ignorance on my part but I don't feel like the two issues are related. I have spent a considerable amount of time looking at the comment from Maurycy Markowski, the #23674 and the dotnet/efcore repo. I know you guys are really busy doing great things and I didn't really want to write this replay but could you help me understand how they are related?

Side Note:
Maurycy Markowski said "which we remove and therefore the "filter" part of the include is completely ignored". The strange thing is this actually not completely ignored and works somewhat. The customerPart in "_modelHeader -> orderParts[0] -> customerParts -> customerPartsHeader -> customerParts[0]" is always the same part as  "_modelHeader -> orderParts[0] -> customerParts". The desired results are always in "_modelHeader -> orderParts[0] -> customerParts -> customerPartsHeader -> customerParts[1]". The query is correct, the problem is in the materialization of the entity. 

image001

@smitpatel smitpatel reopened this Mar 24, 2021
@ajcvickers ajcvickers added this to the 6.0.0 milestone Mar 26, 2021
@maumar maumar removed this from the 6.0.0 milestone Sep 21, 2021
@maumar
Copy link
Contributor

maumar commented Sep 22, 2021

I was wrong and @CTGControls was correct. We don't end up filtering out this cycle. Nav expansion processes the query properly. We get something like this (which contains both Take(1) statements):

DbSet<orderPartsHeader>()
    .Where(o => o.id == __id_0)
    .Select(o => IncludeExpression(
        o, 
        MaterializeCollectionNavigation(
            Navigation: orderPartsHeader.orderParts,
            subquery: DbSet<orderParts>()
                .Where(o0 => EF.Property<Guid?>(o, "id") != null && object.Equals(
                    objA: (object)EF.Property<Guid?>(o, "id"), 
                    objB: (object)EF.Property<Guid?>(o0, "orderPartsHeaderId")))
                .OrderByDescending(o0 => o0.createdOnDate)
                .Take(1)
                .Join(
                    inner: DbSet<customerParts>(), 
                    outerKeySelector: o0 => EF.Property<Guid?>(o0, "customerPartsId"), 
                    innerKeySelector: c => EF.Property<Guid?>(c, "id"), 
                    resultSelector: (o, i) => new TransparentIdentifier<orderParts, customerParts>(
                        Outer = o, 
                        Inner = i
                    ))
                .Join(
                    inner: DbSet<customerPartsHeader>(), 
                    outerKeySelector: o0 => EF.Property<Guid?>(o0.Inner, "customerPartsHeaderId"), 
                    innerKeySelector: c0 => EF.Property<Guid?>(c0, "id"), 
                    resultSelector: (o, i) => new TransparentIdentifier<TransparentIdentifier<orderParts, customerParts>, customerPartsHeader>(
                        Outer = o, 
                        Inner = i
                    ))
                .Select(o0 => IncludeExpression(
                    o0.Outer.Outer, 
                    IncludeExpression(
                        o0.Outer.Inner, 
                        IncludeExpression(
                            o0.Inner, 
                            MaterializeCollectionNavigation(
                                Navigation: customerPartsHeader.customerParts,
                                subquery: DbSet<customerParts>()
                                    .Where(c1 => EF.Property<Guid?>(o0.Inner, "id") != null && object.Equals(
                                        objA: (object)EF.Property<Guid?>(o0.Inner, "id"), 
                                        objB: (object)EF.Property<Guid?>(c1, "customerPartsHeaderId")))
                                    .OrderByDescending(c1 => c1.createdOnDate)
                                    .Take(1)), customerParts)
                        , customerPartsHeader)
                    , customerParts)
                )), orderParts)
    )
    .FirstOrDefault()

and the sql is (also correct):

SELECT [t].[id], [t].[approvedRejectedByIp], [t].[approvedRejectedByUserId], [t].[approvedRejectedOnDate], [t].[changeKey], [t].[deletedByIp], [t].[deletedByUserId], [t].[deletedOnDate], [t].[isActive], [t].[isDeleted], [t].[lastModifiedByIp], [t].[lastModifiedByUserId], [t].[lastModifiedOnDate], [t].[lineNumber], [t].[ordersHeaderId], [t2].[id], [t2].[changeKey], [t2].[createdByIp], [t2].[createdByUserId], [t2].[createdOnDate], [t2].[customerPartsId], [t2].[orderPartsHeaderId], [t2].[orderQty], [t2].[orderWt], [t2].[userfriendlyName], [t2].[id0], [t2].[changeKey0], [t2].[createdByIp0], [t2].[createdByUserId0], [t2].[createdOnDate0], [t2].[customerPartsHeaderId], [t2].[description], [t2].[name], [t2].[userfriendlyName0], [t2].[id1], [t2].[approvedRejectedByIp], [t2].[approvedRejectedByUserId], [t2].[approvedRejectedOnDate], [t2].[changeKey1], [t2].[deletedByIp], [t2].[deletedByUserId], [t2].[deletedOnDate], [t2].[isActive], [t2].[isDeleted], [t2].[lastModifiedByIp], [t2].[lastModifiedByUserId], [t2].[lastModifiedOnDate], [t2].[id2], [t2].[changeKey2], [t2].[createdByIp1], [t2].[createdByUserId1], [t2].[createdOnDate1], [t2].[customerPartsHeaderId0], [t2].[description0], [t2].[name0], [t2].[userfriendlyName1]
FROM (
    SELECT TOP(1) [o].[id], [o].[approvedRejectedByIp], [o].[approvedRejectedByUserId], [o].[approvedRejectedOnDate], [o].[changeKey], [o].[deletedByIp], [o].[deletedByUserId], [o].[deletedOnDate], [o].[isActive], [o].[isDeleted], [o].[lastModifiedByIp], [o].[lastModifiedByUserId], [o].[lastModifiedOnDate], [o].[lineNumber], [o].[ordersHeaderId]
    FROM [orderPartsHeader] AS [o]
    WHERE [o].[id] = @__id_0
) AS [t]
OUTER APPLY (
    SELECT [t0].[id], [t0].[changeKey], [t0].[createdByIp], [t0].[createdByUserId], [t0].[createdOnDate], [t0].[customerPartsId], [t0].[orderPartsHeaderId], [t0].[orderQty], [t0].[orderWt], [t0].[userfriendlyName], [c].[id] AS [id0], [c].[changeKey] AS [changeKey0], [c].[createdByIp] AS [createdByIp0], [c].[createdByUserId] AS [createdByUserId0], [c].[createdOnDate] AS [createdOnDate0], [c].[customerPartsHeaderId], [c].[description], [c].[name], [c].[userfriendlyName] AS [userfriendlyName0], [c0].[id] AS [id1], [c0].[approvedRejectedByIp], [c0].[approvedRejectedByUserId], [c0].[approvedRejectedOnDate], [c0].[changeKey] AS [changeKey1], [c0].[deletedByIp], [c0].[deletedByUserId], [c0].[deletedOnDate], [c0].[isActive], [c0].[isDeleted], [c0].[lastModifiedByIp], [c0].[lastModifiedByUserId], [c0].[lastModifiedOnDate], [t1].[id] AS [id2], [t1].[changeKey] AS [changeKey2], [t1].[createdByIp] AS [createdByIp1], [t1].[createdByUserId] AS [createdByUserId1], [t1].[createdOnDate] AS [createdOnDate1], [t1].[customerPartsHeaderId] AS [customerPartsHeaderId0], [t1].[description] AS [description0], [t1].[name] AS [name0], [t1].[userfriendlyName] AS [userfriendlyName1]
    FROM (
        SELECT TOP(1) [o0].[id], [o0].[changeKey], [o0].[createdByIp], [o0].[createdByUserId], [o0].[createdOnDate], [o0].[customerPartsId], [o0].[orderPartsHeaderId], [o0].[orderQty], [o0].[orderWt], [o0].[userfriendlyName]
        FROM [orderParts] AS [o0]
        WHERE [t].[id] = [o0].[orderPartsHeaderId]
        ORDER BY [o0].[createdOnDate] DESC
    ) AS [t0]
    INNER JOIN [customerParts] AS [c] ON [t0].[customerPartsId] = [c].[id]
    INNER JOIN [customerPartsHeader] AS [c0] ON [c].[customerPartsHeaderId] = [c0].[id]
    LEFT JOIN (
        SELECT [t3].[id], [t3].[changeKey], [t3].[createdByIp], [t3].[createdByUserId], [t3].[createdOnDate], [t3].[customerPartsHeaderId], [t3].[description], [t3].[name], [t3].[userfriendlyName]
        FROM (
            SELECT [c1].[id], [c1].[changeKey], [c1].[createdByIp], [c1].[createdByUserId], [c1].[createdOnDate], [c1].[customerPartsHeaderId], [c1].[description], [c1].[name], [c1].[userfriendlyName], ROW_NUMBER() OVER(PARTITION BY [c1].[customerPartsHeaderId] ORDER BY [c1].[createdOnDate] DESC) AS [row]
            FROM [customerParts] AS [c1]
        ) AS [t3]
        WHERE [t3].[row] <= 1
    ) AS [t1] ON [c0].[id] = [t1].[customerPartsHeaderId]
) AS [t2]
ORDER BY [t].[id], [t2].[createdOnDate] DESC, [t2].[id], [t2].[id0], [t2].[id1], [t2].[customerPartsHeaderId0], [t2].[createdOnDate1] DESC

The sql produces 1 row. Given the seed data provided, this row is used to construct:

  • orderPartsHeader on top level (id = 5ab4cb69-7982-44fa-97cd-03e1d386e5e6)
  • orderPartsHeader's related orderPart (user friendly name = "Order Part 1")
  • orderPart's related customerParts, single entity, since its a reference (user friendly name = "OLD CUSTOMER PART")
  • customerPart's related customerPartsHeader (id = "596efbff-8c30-4b75-bfdb-ef7e05b5f96d")
  • customerPartsHeader's related customerPart (user friendly name = "Second NEW CUSTOMER PART")

That last navigation is a collection, and the results contain TWO customerPart entities. This is due to fixup (described https://docs.microsoft.com/en-us/ef/core/querying/related-data/eager#filtered-include in the "Caution" section). Basically, we materialized the "OLD CUSTOMER PART" earlier in the same query, and later when we materialize collection of customerParts, that earlier customer is added, even though the actual data that we pull only contains info about "Second NEW CUSTOMER PART". If the query is modified, so that the customer Part returned from the second filteredInclude is the same as the one pulled earlier, we get only one element in the customerPart collection correctly.

FFM.DataAccessModels.App.orderPartsHeader _modelHeader = 
    await _FFM_DbContext.orderPartsHeader
        .Where(m => m.id == id)
            .Include(x => x.orderParts.OrderByDescending(x => x.createdOnDate).Take(1))
                .ThenInclude(x => x.customerParts.customerPartsHeader.customerParts.OrderBy/*Descending*/(
                    x => x.createdOnDate).Take(1)
                )
        .FirstOrDefaultAsync(m => m.id == id)
;

@maumar
Copy link
Contributor

maumar commented Sep 22, 2021

Documentation suggests to use NoTracking query, but it doesn't work in this case due to cycle (issue #16225) - we are more diligent in detecting them in this code path and actually throw. Another workaround we propose is to clear out the state manager by using a different dbcontext between queries, but that also can't work here since everything happens in the same query. We should add this example to documentation, but unfortunately I don't think there is any good workaround

@CTGControls
Copy link
Author

CTGControls commented Sep 29, 2021

Our application uses this parent(header)/child architecture for change tracking. We don't update or delete we only create new child items. In the above example we are trying to get the newest revision of "_modelHeader -> orderParts[0] -> customerParts". I know we could make multiple database calls to get this data, but in our architecture the the network latency is the slowest point. So the lowest number DB calls is desired.

We are currently using ADO and Automapper for items that don't work with EF and frankly this is relatively fast. Every version of EF gets us a little closer to full EF usage. Before EF added filtered includes we where doing all filtered includes with ADO now some are able to be done in EF. It would be nice to drop to one ORM technology and fully drop Automapper. It seems from @maumar replay this will not be possible in the near future or at all with EF.

@ajcvickers
Copy link
Member

/cc @maumar

@maumar
Copy link
Contributor

maumar commented Oct 4, 2021

@CTGControls that's correct. We discussed it more within the team (how viable it would be to fix the problem for NoTracking - wrt tracked this is already by design) and it's super hard and realistically won't happen. Splitting into 2 queries is the only real workaround if you want to use EF for this scenario. Given what you said, using alternatives seems like the right call in this case.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
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