-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Initial change tracking for skip navigations #21516
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.
Fixed the translation error arising in #21517 (tested by putting fix on this branch)
test/EFCore.InMemory.FunctionalTests/Query/ManyToManyNoTrackingQueryInMemoryTest.cs
Outdated
Show resolved
Hide resolved
{ | ||
var associationEntry = FindAssociationEntry(entry, otherEntry, skipNavigation); | ||
|
||
if (associationEntry == null) |
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.
I thought we agreed to limit this to entity types that only have the foreign keys
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.
I'm not sure I 100% agreed. ;-) Anyway, I'm going to look into this more, so the code here is not final.
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.
The description says:
Covers only the simple case of collection skips with non-composite, non-alternate keys and simple concrete join table containing only two FK properties
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.
I talked to your manager, and he said it's okay to merge this now and revisit later.
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.
Perhaps I should speak to your manager then 🙃
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.
I would definitely get @smitpatel to look over the query changes. But for my part it looks good.
8918272
to
eef0f1e
Compare
@smitpatel Any more comments on this? |
test/EFCore.Sqlite.FunctionalTests/Query/ManyToManyQuerySqliteTest.cs
Outdated
Show resolved
Hide resolved
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.
Fine to merge. @AndriySvyryd should also take a look when he returns in case I missed anything.
Part of #19003 Covers only the simple case of collection skips with non-composite, non-alternate keys and simple concrete join table containing only two FK properties. Many-to-many tracking queries are enabled, but some are skipped due to query translation errors.
185e37b
to
0a15d43
Compare
Part of #19003
Covers only the simple case of collection skips with non-composite, non-alternate keys and simple concrete join table containing only two FK properties.
Many-to-many tracking queries are enabled, but some are skipped due to query translation errors.