Fix missing subclass discriminator in ON clause for entity joins #2600
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bahusoid
changed the title
bahusoid
changed the title
fredericDelaporte
approved these changes
Nov 11, 2020
|
Should we wait a bit in case of other issues waiting to be reported, or release this soon, like on next Sunday? |
|
I would say don't wait. |
bahusoid
added a commit
to bahusoid/nhibernate-core
that referenced
this pull request
Nov 14, 2020
|
Just in case of troubles with resolving conflicts when merge to master: |
bahusoid
added a commit
to bahusoid/nhibernate-core
that referenced
this pull request
Aug 10, 2021
Comment on lines
+196
to
200
| // Apply filters for entity joins and Many-To-One association | ||
| var enabledForManyToOne = FilterHelper.GetEnabledForManyToOne(enabledFilters); | ||
| condition = new SqlString(string.IsNullOrEmpty(on) && enabledForManyToOne.Count > 0 | ||
| condition = new SqlString(string.IsNullOrEmpty(on) && (ForceFilter || enabledForManyToOne.Count > 0) | ||
| ? join.Joinable.FilterFragment(join.Alias, enabledForManyToOne) | ||
| : on); |
There was a problem hiding this comment.
@bahusoid,
I think we have missed something here, which could have fixed #3046 if done similarly in other places having that ForceFilter: when true, shouldn't it filter on all enabled filters, not just on many-to-one enabled ones ?
// Apply filters for entity joins and Many-To-One association
var enabledFiltersForJoin = ForceFilter ? enabledFilters : FilterHelper.GetEnabledForManyToOne(enabledFilters);
condition = new SqlString(string.IsNullOrEmpty(on) && (enabledFiltersForJoin.Count > 0)
? join.Joinable.FilterFragment(join.Alias, enabledFiltersForJoin)
: on);
But it may not be enough, because the on variable construction does exclude filters not enabled for many-to-one too.
Comment on lines
+855
to
858
| // Apply filters for entity joins and Many-To-One associations | ||
| if (oj.ForceFilter || enabledFiltersForManyToOne.Count > 0) | ||
| { | ||
| string manyToOneFilterFragment = oj.Joinable.FilterFragment(oj.RHSAlias, enabledFiltersForManyToOne); |
There was a problem hiding this comment.
Similarly:
// Apply filters for entity joins and Many-To-One associations
var enabledFiltersForJoin = oj.ForceFilter ? enabledFilters : enabledFiltersForManyToOne;
if (oj.ForceFilter || enabledFiltersForJoin.Count > 0)
{
string manyToOneFilterFragment = oj.Joinable.FilterFragment(oj.RHSAlias, enabledFiltersForJoin);
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This issue affects both Criteria/QueryOver and Hql/Linq Query API. While it's regression only for LINQ case I made fix also for Criteria (as fix is quite simple).
Fixes #2599 (complete fix for #2463)