Inconsistent ordering in SQL queries generated for split query with Include

[giangpham712]

The tests in in NorthwindSplitIncludeNoTrackingQueryTestBase

Include_collection_skip_no_order_by
Include_collection_take_no_order_by
Include_collection_skip_take_no_order_by

when being run against CockroachDB (a PostgreSQL-like database) using efcore.pg library https://github.com/npgsql/efcore.pg/blob/main/test/EFCore.PG.FunctionalTests/Query/NorthwindSplitIncludeNoTrackingQueryNpgsqlTest.cs

generates split queries with inconsistent ordering

For example, the test Include_collection_take_no_order_by

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Include_collection_take_no_order_by(bool async)
      => AssertQuery(
      async,
      ss => ss.Set<Customer>().Take(10).Include(c => c.Orders),
      elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude<Customer>(c => c.Orders)),
      entryCount: 110);

generates two queries

SELECT c."CustomerID", c."Address", c."City", c."CompanyName", c."ContactName", c."ContactTitle", c."Country",
c."Fax", c."Phone", c."PostalCode", c."Region"
 FROM "Customers" AS c
 ORDER BY c."CustomerID" NULLS FIRST
 LIMIT 10;

and

 SELECT o."OrderID", o."CustomerID", o."EmployeeID", o."OrderDate", t."CustomerID"
 FROM (SELECT c."CustomerID"
 FROM "Customers" AS c
 LIMIT 10) AS t INNER
   JOIN "Orders" AS o
    ON t."CustomerID" = o."CustomerID"
 ORDER BY t."CustomerID" NULLS FIRST;

While the first query has ORDER BY clause added automatically the second query uses results from a subquery without a corresponding ORDER BY clause. This causes the second query to return orders that don't match with the customers returned by the first query. As a result, these tests will fail because some Customer objects don't have the expected Order objects.

Test failure:

Include_collection_take_no_order_by(async: False)

Xunit.Sdk.EqualException
Assert.Equal() Failure
Expected: 14
Actual:   0
   at Microsoft.EntityFrameworkCore.TestUtilities.QueryAsserter.AssertIncludeCollection[TElement](IEnumerable`1 expected, IEnumerable`1 actual, IEnumerable`1 expectedIncludes, Boolean assertOrder)
   at InvokeStub_QueryAsserter.AssertIncludeCollection(Object, Object, IntPtr*)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

The missing of ORDER BY clause in second query may not affect PostgreSQL but it affects CockroachDB because ordering in CockroachDB is non-deterministic without the ORDER BY clause

EF Core version: 8.0.0-preview.4.23259.3
Database provider: PostgreSQL, CockroachDB
Target framework: net8.0
Operating system: macOS
IDE: JetBrains Rider 2023.1.2

[roji]

@giangpham712 I dug into this a bit, and I indeed think there's a more general problem in this area that's not specific to CockroachDB. We've discussed this before in the past, I've summarized the situation in #26808 (comment) and will discuss with the team.

In the meantime, I suggest skipping any test which fails because of Skip/Take without OrderBy when split query is in use (or asserting failure instead of skipping, which is what we do in EF generally). Keep your eyes on #26808 for further progress.

[roji]

Duplicate of #26808

[giangpham712]

Thanks @roji