-
Notifications
You must be signed in to change notification settings - Fork 931
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
Use table group joins for many-to-many in Criteria and Entity loaders #2687
Use table group joins for many-to-many in Criteria and Entity loaders #2687
Conversation
b6c53fa
to
bc7e5c3
Compare
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.
When testing with the new tests but without NHibernate changes, only the queryover test in NH750 fails. All other tests succeeds, included the lazy loading one. Your description seems to imply the "lazy load with throws for not found" test should have failed without your change.
Could you rebase this branch? It appears to be based on a quite old master dating back to the the start of the year.
src/NHibernate.Test/NHSpecificTest/GH1994/ManyToManyFilteredFixture.cs
Outdated
Show resolved
Hide resolved
src/NHibernate/Persister/Collection/AbstractCollectionPersister.cs
Outdated
Show resolved
Hide resolved
src/NHibernate/Persister/Collection/AbstractCollectionPersister.cs
Outdated
Show resolved
Hide resolved
69d1442
to
a0a4dd2
Compare
733e8e2
to
0bafa66
Compare
Good catch. Indeed, lazy loading is not affected by this bug as
I updated description. |
438175b
to
a59b6d0
Compare
Cons: query is executed for each not found element for not filtered collection for Criteria (so not ideal when
not-found="ignore"
). But it's the same behavior for LINQ and lazy loading. I will try to address it in separate PR (hibernate uses inner join for many-to-many lazy loading so basically always ignores not found records)Pros: Removes some hacks (like
BasicCollectionPersister.ManyToManySelectFragment
) and unifies behavior between LINQ, Criteria and entity loaders.Possible breaking change: Default
not-found
behavior now works correctly so now it throwsObjectNotFoundException
exception on many-to-many Criteria fetch for not found records.