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

Fix to #35007 - EF Core fails to translate LINQ query to SQL if JOIN contains multiple columns #35019

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Oct 30, 2024

Problem was in the GroupJoinConvertingExpressionVisitor where we convert GroupJoin key selectors into the correlation predicate for result selector subqueries. We blindly copy them over and in case of composite key selectors which fail to translate later in the pipeline (can't translate equals on anonymous object that is not a constant). Normally when processing regular joins we have logic that breaks up anonymous objects into constituent parts and compare them one by one instead (CreateJoinPredicate). However by the time we get to the translation we are dealing with a subquery with a Where predicate - we don't know at that point that it came from GroupJoin.

Fix is to tweak the logic in GroupJoinConvertingExpressionVisitor to break apart anonymous objects, like we do in CreateJoinPredicate

Fixes #35007

@maumar maumar requested a review from a team as a code owner October 30, 2024 10:07
@maumar maumar requested review from a team and removed request for a team October 30, 2024 10:07
@maumar maumar force-pushed the fix35007 branch 2 times, most recently from b955d67 to 0754221 Compare November 1, 2024 01:27
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

which fail to translate later in the pipeline (can't translate equals on anonymous object that is not a constant).

Wow, I was just doing a very deep dive into NewExpression handling as part of #35038!

My current thinking is that we should actually "translate" NewExpression in RelationalSqlTranslatingExpressionVisitor, including when it's not a constant. Of course, there's no actual SQL translation to NewExpression, but then we support StructuralTypeShaperExpression RelationalSqlTranslatingExpressionVisitor (so that we can bind properties on it) - so we already support handling non-SQL expression types in that visitor. This would allow us to:

  • Cleanly handle member access over NewExpression in RelationalSqlTranslatingExpressionVisitor, without any hacks (this is currently simplified away in... ReplacingExpressionVisitor...)
  • Not have special handling logic for NewExpression in RelationalProjectionBindingExpressionVisitor (for Select(b => new { })), in TranslateGroupBy (for GroupBy(b => new { }))
  • Go in the direction of allowing users to express row value syntax (a,b) > (1,2) via anonymous objects (Allow users to express SQL row value comparison syntax #26822).
  • Support equality of NewExpression (Where(b => new { b.X, b.Y } == new { c.X, c.Y }). I don't think it's a specific goal to specifically support this somewhat convoluted syntax (though it's valid); but it should allow handling the GroupJoin problem in this issue without the special handling you're adding.

In any case, that's all thoughts and plans - I think it's fine to merge this, and then if all goes well I'll remove the code when I add support for NewExpression comparison.

…contains multiple columns

problem is in the GroupJoinConvertingExpressionVisitor where we convert GroupJoin key selectors into the correlation predicate for result selector subqueries. We blindly copy them over and in case of composite key selectors which fail to translate later in the pipeline (can't translate equals on anonymous object that is not a constant). Normally when processing regular joins we have logic that breaks up anonymous objects into constituent parts and compare them one by one instead (CreateJoinPredicate). However by the time we get to the translation we are dealing with a subquery with a Where predicate - we don't know at that point that it came from GroupJoin.

Fix is to tweak the logic in GroupJoinConvertingExpressionVisitor to break apart anonymous objects, like we do in CreateJoinPredicate

Fixes #35007
@maumar maumar merged commit d3a4fd2 into main Nov 5, 2024
7 checks passed
@maumar maumar deleted the fix35007 branch November 5, 2024 23:05
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.

EF Core fails to translate LINQ query to SQL if JOIN contains multiple columns
2 participants