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

Adding docs for filtered include feature and updating query samples #2260

Merged
merged 1 commit into from
Apr 10, 2020

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Apr 8, 2020

No description provided.

@maumar maumar requested a review from a team April 8, 2020 21:04
Alternatively, identical operations can be applied for each navigation that is included multiple times.

[!code-csharp[Main](../../../samples/core/Querying/RelatedData/Sample.cs#MultipleLeafIncludesFiltered2)]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many of the filtered include scenarios translate to apply, so sqlite will throw - should we add a warning/caution for that here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for filtered includes, take works in sqlite right?

Copy link
Contributor Author

@maumar maumar Apr 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope -

SELECT "l"."Id", "l"."Date", "l"."Name", "l"."OneToMany_Optional_Self_Inverse1Id", "l"."OneToMany_Required_Self_Inverse1Id", "l"."OneToOne_Optional_Self1Id", "t"."Id", "t"."Date", "t"."Level1_Optional_Id", "t"."Level1_Required_Id", "t"."Name", "t"."OneToMany_Optional_Inverse2Id", "t"."OneToMany_Optional_Self_Inverse2Id", "t"."OneToMany_Required_Inverse2Id", "t"."OneToMany_Required_Self_Inverse2Id", "t"."OneToOne_Optional_PK_Inverse2Id", "t"."OneToOne_Optional_Self2Id"
FROM "LevelOne" AS "l"
OUTER APPLY (
    SELECT "l0"."Id", "l0"."Date", "l0"."Level1_Optional_Id", "l0"."Level1_Required_Id", "l0"."Name", "l0"."OneToMany_Optional_Inverse2Id", "l0"."OneToMany_Optional_Self_Inverse2Id", "l0"."OneToMany_Required_Inverse2Id", "l0"."OneToMany_Required_Self_Inverse2Id", "l0"."OneToOne_Optional_PK_Inverse2Id", "l0"."OneToOne_Optional_Self2Id"
    FROM "LevelTwo" AS "l0"
    WHERE "l"."Id" = "l0"."OneToMany_Optional_Inverse2Id"
    ORDER BY "l0"."Name"
    LIMIT 3
) AS "t"
ORDER BY "l"."Id", "t"."Name", "t"."Id"

Where works, OrderBy.AsQueryable works, Skip/Take fails due to APPLY, naked OrderBy fails due to a bug

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something is wrong then. We added whole support for RowNumberExpression to convert apply to join when there is only take. dotnet/efcore#17293
There is also test which converts Take(4) in select many. At the minimum query you posted above has nothing in it which stops it from getting converted to join.

Also related to it: If we fix dotnet/efcore#17326, then skip would also convert to number and most of it should get translated to Sqlite. (For filtered include case, everything would work).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will investigate what's up

Copy link
Contributor Author

@maumar maumar Apr 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctx.LevelOne.Select(l1 => l1.OneToMany_Optional1.OrderBy(x => x.Name).Take(3).ToList()).ToList();

still translates to apply on sqlite (and fails).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File an issue and don't add any warning here.

entity-framework/core/querying/related-data.md Outdated Show resolved Hide resolved
entity-framework/core/querying/related-data.md Outdated Show resolved Hide resolved
entity-framework/core/querying/related-data.md Outdated Show resolved Hide resolved
entity-framework/core/querying/related-data.md Outdated Show resolved Hide resolved
entity-framework/core/querying/related-data.md Outdated Show resolved Hide resolved
@maumar maumar force-pushed the filtered_include_docs branch from 8f99478 to 19999ba Compare April 10, 2020 00:39
@maumar maumar force-pushed the filtered_include_docs branch from 19999ba to 7012051 Compare April 10, 2020 00:41
entity-framework/core/querying/related-data.md Outdated Show resolved Hide resolved
entity-framework/core/querying/related-data.md Outdated Show resolved Hide resolved
entity-framework/core/querying/related-data.md Outdated Show resolved Hide resolved
@maumar maumar force-pushed the filtered_include_docs branch from 7012051 to 74d0d46 Compare April 10, 2020 18:11
@maumar
Copy link
Contributor Author

maumar commented Apr 10, 2020

updated

@maumar maumar merged commit 74d0d46 into master Apr 10, 2020
@smitpatel smitpatel deleted the filtered_include_docs branch April 10, 2020 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants