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

Only includes relations eagerly included #93

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

zoriya
Copy link
Contributor

@zoriya zoriya commented Nov 6, 2023

On #90 and #92, I added navigation queries on the last .Select call to keep included properties on the returned entities, but doing so actually made EF fetch all relations of this item. This PR actually reads .Include calls to prevent unnecessary fetching.

With an example:

db.Set<User>()
	.Include(x => x.Orders)
	.First();

resulted in

db.Set<User>()
	.Include(x => x.Orders)
	.Select(x => new User{
		Id = x.Id,
		Name = x.Name,
		Orders = x.Orders,
		Friends = x.Friends // Here relation friends is fetched but it was not requested. This can create really slow queries.
	})
	.First();

and after this PR this generates:

db.Set<User>()
	.Include(x => x.Orders)
	.Select(x => new User{
		Id = x.Id,
		Name = x.Name,
		Orders = x.Orders,
		// No Friends requeseted here.
	})
	.First();

Filtered includes (like in the example below) are not parsed, and I don't know how they should be handled.

db.Set<User>()
	.Incude(x => x.Orders.Where(x => x.Price > 100));

I also made root query rewriting use properties directly instead of their baking fields (it was way simpler than I thought).

Based on #91 since include parsing was needed and was already done.

@zoriya
Copy link
Contributor Author

zoriya commented Nov 6, 2023

Also, something I just found out, but query can't be tracked when root rewriting is enabled. Something we might want to consider to decide if OnlyOnInclude should default to true or false.

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.

1 participant