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

[6.0.2] Query: Assign proper type to CollectionResultExpression #27134

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

smitpatel
Copy link
Contributor

We copied type from subquery itself which is always Queryable type because of conversion we do. We need to convert it back to enumerable.
Issue happens only when nested level because we use element type so for 1 level queryable is gone. But for nested level we get Queryable as element type on outer which is not assignable from List
The conversion doesn't cause issue with user having Queryable in the result since that throws exception much earlier.

Resolves #27105

Description
When projecting enumerable of enumerable, we didn't assign correct type to expression tree when translating causing it to have incorrect type which later fails when generating compiled code.

Customer impact
Customers with enumerable of enumerable in projection will have query failing.

How found
Customer reported on 6.0.1

Regression
Yes, From 5.0

Testing
Added test for user scenario.

Risk
Very low risk. Also added quirk.

@smitpatel smitpatel requested a review from a team January 6, 2022 22:52
@smitpatel smitpatel changed the title Query: Assign proper type to CollectionResultExpression [release/6.0] Query: Assign proper type to CollectionResultExpression Jan 6, 2022
@AndriySvyryd AndriySvyryd added this to the 6.0.x milestone Jan 7, 2022
@AndriySvyryd AndriySvyryd modified the milestones: 6.0.x, 6.0.2 Jan 7, 2022
We copied type from subquery itself which is always Queryable type because of conversion we do. We need to convert it back to enumerable.
Issue happens only when nested level because we use element type so for 1 level queryable is gone. But for nested level we get Queryable<T> as element type on outer which is not assignable from List<T>
The conversion doesn't cause issue with user having Queryable in the result since that throws exception much earlier.

Resolves #27105
@smitpatel smitpatel merged commit 100b207 into release/6.0 Jan 10, 2022
@smitpatel smitpatel deleted the smit/ListOfList branch January 10, 2022 18:08
@smitpatel smitpatel removed this from the 6.0.2 milestone Jan 10, 2022
@ajcvickers ajcvickers changed the title [release/6.0] Query: Assign proper type to CollectionResultExpression [6.0.2] Query: Assign proper type to CollectionResultExpression Jan 13, 2022
smitpatel added a commit that referenced this pull request Mar 17, 2022
This improves the fix made in #27134

In earlier PR we converted `IQueryable<T>` to `IEnumerable<T>` becaused we converted enumerable to queryable during preprocessing phase and types aligned in later phase since we create a list (which does implement `IEnumerable<T>`). Though this implementing behavior doesn't work when returning a single result of enumerable during async which wraps collection type inside Task.
The better fix is to assign `List<T>` as type since we are eventually creating a list.
In case of single result, the single result operator has generic type which will introduce convert node into it automatically which will match types for async tasks.

Resolves #27600
smitpatel added a commit that referenced this pull request Mar 17, 2022
This improves the fix made in #27134

In earlier PR we converted `IQueryable<T>` to `IEnumerable<T>` becaused we converted enumerable to queryable during preprocessing phase and types aligned in later phase since we create a list (which does implement `IEnumerable<T>`). Though this implementing behavior doesn't work when returning a single result of enumerable during async which wraps collection type inside Task.
The better fix is to assign `List<T>` as type since we are eventually creating a list.
In case of single result, the single result operator has generic type which will introduce convert node into it automatically which will match types for async tasks.

Resolves #27600
dougbu pushed a commit that referenced this pull request Apr 5, 2022
…27664)

This improves the fix made in #27134

In earlier PR we converted `IQueryable<T>` to `IEnumerable<T>` becaused we converted enumerable to queryable during preprocessing phase and types aligned in later phase since we create a list (which does implement `IEnumerable<T>`). Though this implementing behavior doesn't work when returning a single result of enumerable during async which wraps collection type inside Task.
The better fix is to assign `List<T>` as type since we are eventually creating a list.
In case of single result, the single result operator has generic type which will introduce convert node into it automatically which will match types for async tasks.

Resolves #27600
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants