Fix ICriteria/QueryOver create incorrect left join condition when table-per-hierarchy is used with filters

[froedep]

Fixes #1366

[oskarb]

Proposed fix seems non-obvious to me. Doesn't this move all many-to-one filter fragments to the join clause? Is that really the intention? If filters aren't enabled, the modified code isn't even used - might the true problem be somewhere else, where the enabling of the filter causes the join condition to be miss-classified?

[oskarb]

If the filter isn't enabled, the condition on the department discriminator isn't included at all in the sql.

[oskarb]

A lot of confusion seems to exist around the filter on many-to-one feature, and it seems far from clear what the intended/useful behavior would be. Therefore I don't want to change anything at the moment. See further comments in NH-3506, NH-1179, NH-1293, NH-2877, NH-1930, NH-3461 and various historic mailing list discussions.

[froedep]

The issue here isn't the filters themselves - I do not wish to apply a filter to the many-to-one relationship and agree that filters don't make much sense in that scenario.

The problem is that as soon as any filter is enabled (whether used or not) forces the filter conditions (including the discriminator) to be added to the WHERE clause.

So the base query with QueryOver looks like this. Interestingly, no discriminator filtering is applied on the joined class.

FROM   BaseClass this_
       left outer join BaseClass department1_
         on this_.Department = department1_.Id
WHERE  this_.discriminator = 'Emp'

Note, in this next example I have a filter enabled but it's not used in the query.

FROM   BaseClass this_
       left outer join BaseClass department1_
         on this_.Department = department1_.Id
WHERE  department1_.discriminator = 'Dep'
       and this_.discriminator = 'Emp'

This effectively turns a left outer join into an inner join as the join conditions should be on the JOIN itself.

Interestingly, HQL appears to generate SQL exactly as I would expect it to. Even more interesting, the SQL produced by HQL is identical when a filter is enabled (but not used) as well as when no filters are enabled.

from   BaseClass employee0_
       left outer join BaseClass department1_
         on employee0_.Department = department1_.Id
            and department1_.discriminator = 'Dep'
where  employee0_.discriminator = 'Emp'

[froedep]

Further to your comment on JIRA, I have set UseInManyToOne to false and it does indeed cause ICriteria/QueryOver to produce the same SQL as previously with no filters enabled.

• Should this perhaps be the default (or as you say, filters removed completely from one-to-many)
• Having a filter enabled does NOT affect any many-to-one joins - the discriminator is not applied in the same way as on one-to-many queries without filters enabled.
• There is still a discrepancy as ICriteria/QueryOver show a breaking difference when a seemingly unrelated thing is triggered whereas HQL simply does the right thing.

[hazzik]

Obviously this was a typo (or maybe misunderstanding of what it will do, as it is not really obvious), to add AddCondition in the first place as the original code already checks that filter has applied to ToFromFragmentString, not to ToWhereFragmentString.

The proper fix to the feature is to remove if (enabledFiltersForManyToOne.Count > 0) statement, and make this block unconditional.

But... some feelings say to me, that this code is just in incorrect place.

[ithielnor]

Does this solution still add the discriminator to where clause for the root table?

[hazzik]

Can you please enable Allow edits from maintainers PR option?

[froedep]

@hazzik assuming you meant me, I have enabled edits from maintainers. Thanks.

[hazzik]

NB: Generate async

[hazzik]

This is just fixing a pure bug in the query. The patch is correct. However, there is a potential for a fix for a complex problem of filters.

[fredericDelaporte]

As I analyse this, the root trouble of the issue is the undue addition of the discriminator condition to many-to-one when filters are enabled. This condition is not wrong but is unneeded, since we anyway join on the association id, and the id is enough alone to get the proper row in the joined table.
Furthermore, this undue condition is added in the where clause, which changes the query semantic in case of a left join.

The fix here does not fix that root trouble, which would likely require quite more work and is maybe not worth it.
It addresses instead the place where the condition is added, avoiding the query semantic change. And as said by Alexander, it should also fix some filter cases which could be wrongly put in the where instead of the join.

So for the reported case, this fix allows getting correct results, although with a slightly less efficient query than what we could have. Since I do not think fixing the root trouble would be simple, it is good enough for me.

[hazzik]

the root trouble of the issue is the undue addition of the discriminator condition to many-to-one when filters are enabled.

This is not a root trouble, this is just an additional small problem, which produces sub-optimal query.