Fix fetching lazy component on not mapped interface

[bahusoid]

Fixes #3289

not a regression but let's fix in 5.4

[hazzik]

We usually traverse all possible implementations in other places, not just select first.

…

On Sat, 17 Jun 2023 at 3:49 PM, Roman Artiukhin ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/NHibernate/Linq/Visitors/ResultOperatorProcessors/ProcessFetch.cs <#3320 (comment)> : > @@ -52,6 +52,16 @@ public void Process(FetchRequestBase resultOperator, QueryModelVisitor queryMode { var metadata = queryModelVisitor.VisitorParameters.SessionFactory .GetClassMetadata(resultOperator.RelationMember.ReflectedType); + if (metadata == null) + { + var entityName = queryModelVisitor.VisitorParameters.SessionFactory.GetImplementors( + resultOperator.RelationMember.ReflectedType.FullName).SingleOrDefault(); For some reasons I didn't think it's a valid scenario. Fixed. — Reply to this email directly, view it on GitHub <#3320 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABDLF7T25D3YPMFQ6GXK2DXLVAOBANCNFSM6AAAAAAZI3JI4M> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[bahusoid]

We usually traverse all possible implementations in other places, not just
select first.

Nope. And why we need to do it here? Other place with explanation:

nhibernate-core/src/NHibernate/Util/ExpressionsHelper.cs

Lines 498 to 503 in 90d50c0

	// When dealing with a polymorphic query it is not important which entity name we pick
	// as they all need to have the same mapped types for members of the type that is queried.
	// If one of the entites has a different type mapped (e.g. enum mapped as string instead of numeric),
	// the query will fail to execute as currently the ParameterMetadata is bound to IQueryPlan and not to IQueryTranslator
	// (e.g. s.Query<IEntity>().Where(a => a.MyEnum == MyEnum.Option)).
	currentEntityName = sessionFactory.GetImplementors(currentEntityName).FirstOrDefault();

Ideally we should use ExpressionsHelper.TryGet methods but it's too early for it...

[hazzik]

Ok, thanks for the clarification. Another question, why do we fix it in 5.4.x? Was it a regression?

[bahusoid]

Was it a regression?

No (it's mentioned in pr description). But fix is simple so why not?

[fredericDelaporte]

There is always a risk of introducing some new regression with each change. Ideally 5.4.x releases are meant to stabilize (fix regressions) the 5.4 minor version. Additional fixes added into it may delay its stabilization. And also, adding new fixes in it instead of vNext (5.5) is not an incentive to finish 5.5.

That said, sure, the fix seems to come with very little risks of unexpected consequences, and we have already accepted other little additional fixes in the past.