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.

Sign up for GitHub

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

Jump to bottom

Query: SelectExpression.Update always creates new instance of SelectExpression without checking if anything changed #31276

Closed
maumar opened this issue Jul 15, 2023 · 1 comment · Fixed by #32817
Closed

Query: SelectExpression.Update always creates new instance of SelectExpression without checking if anything changed #31276

maumar opened this issue Jul 15, 2023 · 1 comment · Fixed by #32817
Assignees
@roji
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-cleanup type-enhancement
Milestone
9.0.0

Comments

@maumar
Copy link
Contributor

maumar commented Jul 15, 2023 •
edited
Loading

This is not how any other Update methods on SqlExpression work. We should figure out if it was intentional and if it's safe to make changes. This may cause functional changes because Equals method on SelectExpression is (intentionally) implemented as ReferenceEquals. We could be missing some optimizations by "updating" the same SelectExpression in effect creating a copy, that we can't recognize as being equivalent anymore.
@roji
Copy link
Member

roji commented Jul 16, 2023

I suspect that we currently use Update() only in cases where we know we're going to change the expression anyway... But I think you're right and we should look into it...

@AndriySvyryd AndriySvyryd added this to the Backlog milestone Jul 19, 2023
@roji roji self-assigned this Jan 14, 2024
@roji roji removed this from the Backlog milestone Jan 14, 2024
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jan 14, 2024
roji added a commit to roji/efcore that referenced this issue Jan 14, 2024
@roji
Move all pruning code to the pruner and make it an immutable visitor
a15c626 
Fixes dotnet#31276
@roji roji mentioned this issue Jan 14, 2024
Merged
@ajcvickers ajcvickers added this to the 9.0.0 milestone Jan 17, 2024
roji added a commit to roji/efcore that referenced this issue Jan 20, 2024
@roji
Move all pruning code to the pruner and make it an immutable visitor
0091907 
Fixes dotnet#31276
roji added a commit to roji/efcore that referenced this issue Jan 20, 2024
@roji
Move all pruning code to the pruner and make it an immutable visitor
9ee2bd5 
Fixes dotnet#31276
roji added a commit that referenced this issue Jan 20, 2024
@roji
Move all pruning code to the pruner and make it an immutable visitor (#…
def705a 
…32817)

Fixes #31276
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview1 Jan 31, 2024
@roji roji modified the milestones: 9.0.0-preview1, 9.0.0 Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@roji roji

Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-cleanup type-enhancement
Projects
None yet
Milestone
9.0.0
Development

Successfully merging a pull request may close this issue.

Move all pruning code to the pruner and make it an immutable visitor roji/efcore
4 participants
@ajcvickers @maumar @roji @AndriySvyryd