Include the query in loader PostInstantiate QueryException

[carlileconsult]

I found it difficult when updating an older application using an older version of NHibernate, to track down what entity relationship was causing the QueryException "Cannot simultaneously fetch multiple bags.". When I updated the exception to return the entityName I found it extremely easy to identify the entity that needed changing.

[fredericDelaporte]

This looks a bit hackish. Why not using something like $"Cannot simultaneously fetch multiple bags for query {ToString()}"?

[carlileconsult]

Thanks for taking the time to review and comment. I appreciate the feedback.

With the use of a string interpolation, the statement is automatically converted to string.format(). This methods second parameter is of type params object[], which upcasts all values to object before invoking ToString().

I was a little concerned about the accessing of an array directly, fearing an "object reference not set to an instance of an object". But, with the if check on bagCount I don't believe it would ever reach the line without having something.

Maybe it would be safer to capture the EntityName during evaluation of the persisters? I could rework it that way if you prefer?

[fredericDelaporte]

I mean, better use the already overridden ToString() method available in the class (see the base class, Loader), and meant for describing the query. (It could also be $"Cannot simultaneously fetch multiple bags for query {this}", if you do not want to write a ToString() in string interpolation.)

I have not checked if it is possible to load multiple collections without loading an entity. The code right before this throw does not guarantee the entity persister will always have at least one element, so maybe there is a risk here, even if unlikely. But why should we take it, while we already have a method meant for describing the query?

Moreover, in multiple fetches cases, the first entity persister may not be the actual owner of the bags: the message would be then a bit misleading.

[carlileconsult]

That's a great point. Sorry, I didn't realize before, but I understand now. I will work on changing the pull request, once I test it out.

Thanks for taking the time to explain further.

[fredericDelaporte]

.vs is fully ignored in src\.gitignore. Why adding these rules here?