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

Query: correlated collection optimization is broken for queries containing top level First/FirstOrDefault/Single/SingleOrDefault #10813

Closed
maumar opened this issue Jan 30, 2018 · 6 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented Jan 30, 2018

Repro:

var result = ctx.Parents.Select(p =>
new
{
    Children = p.Children
}).FirstOrDefault();

Problem here is that when we rewrite QM to create query that fetches inner elements, we produce join between inner elements and cloned parent query model. However, if parent model contains result operators that change cardinality, we would clone it also and hence produce invalid QM.

@maumar maumar self-assigned this Jan 30, 2018
maumar added a commit that referenced this issue Jan 31, 2018
…or queries containing top level First/FirstOrDefault/Single/SingleOrDefault

Problem was that correlated collection optimization rewrite wasn't properly handling result operators that could be part of the parent query model. Parent QM is cloned, rewritten and then used as part of the inner collection query.

Fix is to properly compensate for result operators that change the cardinality of the result (specifically First/Single/Last), similarly to what we do in the include pipeline.
@ajcvickers ajcvickers added this to the 2.1.0 milestone Feb 2, 2018
@smitpatel
Copy link
Contributor

In the presence of Single value from sequence result operator, we should not be doing any optimization. Rather than we should just use N+1 mode of evaluation since it would run 2 queries anyway. It gives us benefit that 2nd query is filtered based on a parameter rather than a join (also no order by is needed in 2nd query) which would be efficient than running a normal collection query.

@maumar
Copy link
Contributor Author

maumar commented Feb 5, 2018

@smitpatel correlated collections optimization doesn't work if the outer is using outer parameter, so in case of multi-level correlation we would be producing inefficient queries. We could make the optimization you suggest once/if #10877 is addressed

maumar added a commit that referenced this issue Feb 5, 2018
…or queries containing top level First/FirstOrDefault/Single/SingleOrDefault

Problem was that correlated collection optimization rewrite wasn't properly handling result operators that could be part of the parent query model. Parent QM is cloned, rewritten and then used as part of the inner collection query.

Fix is to properly compensate for result operators that change the cardinality of the result (specifically First/Single/Last), similarly to what we do in the include pipeline.
@maumar
Copy link
Contributor Author

maumar commented Feb 5, 2018

fixed in d787c13

@maumar maumar closed this as completed Feb 5, 2018
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 5, 2018
@smitpatel smitpatel reopened this Feb 11, 2018
@smitpatel
Copy link
Contributor

Re-opening this. This is partially fixed. We need tracking issue to do outer parameter injection for single record collection query. Either we use this or file a new one. For include pipeline (perhaps older one) #2182

@maumar
Copy link
Contributor Author

maumar commented Feb 12, 2018

@smitpatel #10877 is already tracking this (see one of the comments)

@maumar
Copy link
Contributor Author

maumar commented Feb 28, 2018

Workaround:

First/FirstOrDefault:

var result = ctx.Parents.Select(p =>
new
{
    Children = p.Children
}).Take(1).ToList().FirstOrDefault();

Single/SingleOrDefault:

var result = ctx.Parents.Select(p =>
new
{
    Children = p.Children
}).Take(2).ToList().SingleOrDefault();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

3 participants