-
Notifications
You must be signed in to change notification settings - Fork 225
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
Execute update sync #2476
Execute update sync #2476
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @smitpatel, couldn't sleep so went ahead and synced against dotnet/efcore#28834 (and dotnet/efcore#28782).
The original failing test passes, but there are three others which fail - see below. I pasted the actual emitted SQL into the baseline for you.
"""); | ||
} | ||
|
||
public override async Task Update_with_cross_join_cross_apply_set_constant(bool async) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fails with invalid reference to FROM-clause entry for table "c"
@@ -1121,6 +1210,72 @@ public override async Task Update_with_outer_apply_set_constant(bool async) | |||
WHERE c.""CustomerID"" = t0.""CustomerID"""); | |||
} | |||
|
|||
public override async Task Update_with_cross_join_left_join_set_constant(bool async) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fails with invalid reference to FROM-clause entry for table "c"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this does pass for Sqlite fine. (I guess Sqlite decided to defer from npgsql here). So looks like npgsql cannot use the table being updated in anywhere except for where predicate. This test is using it in join predicate for left join. This is something you may need to handle in provider given difference from Sqlite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK @smitpatel, opened #2478 to track doing this on my side (will ping you when I get to it), and approved dotnet/efcore#28834. Thanks for looking at this.
"""); | ||
} | ||
|
||
public override async Task Update_with_cross_join_outer_apply_set_constant(bool async) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fails with invalid reference to FROM-clause entry for table "c"
@@ -1083,7 +1167,7 @@ public override async Task Update_with_cross_join_set_constant(bool async) | |||
WHERE c.""CustomerID"" LIKE 'F%'"); | |||
} | |||
|
|||
[ConditionalTheory(Skip = "invalid reference to FROM-clause entry for table c")] | |||
// [ConditionalTheory(Skip = "invalid reference to FROM-clause entry for table c")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer fails 🎉
Given #2508 is already merged, is there anymore blocking issues in core ExecuteUpdate functionality we need to get in rc2? |
Not AFAIK, thanks... There's still #2478 remaining but that's on my side. |
No description provided.