NH-3303: Refactoring of custom SQL code

[ggeurts]

Major refactoring of custom SQL code to reduce code duplication and simplify maintenance and bug fixes.

The following classes have been affected mostly:

• NHibernate.Loader.Custom.Sql.SqlQueryReturnProcessor

  This class had 2 responsibilities. 1) It provided a lookup mechanism for persisters, suffixes and user-defined property result maps, based on raw information read from INativeSQLQueryReturn instances. 2) It transformed INativeSQLQueryReturn instances into NHibernate.Loader.Custom.IReturn instances for use by the NHibernate.Loader.Custom.CustomLoader.

  The code related to the first responsibility has been moved into the new NHibernate.Loader.Custom.Sql.SqlQueryContext class. The code related to the second responsibility has been integrated into the NHibernate.Loader.Custom.Sql.SqlCustomQuery class.

• NHibernate.Loader.Custom.NonScalarReturn

  The return transformation code that originally resided in NHibernate.Loader.Custom.Sql.SqlQueryReturnProcessor and now is part of NHibernate.Loader.Custom.Sql.SqlCustomQuery, was greatly simplified by removing all IReturn implementations that extend NonScalarResult and make the NonScalarResult class a bit smarter.

• NHibernate.Loader.Custom.CustomQuery

  Removed code that adapted lookup functionality of NHibernate.Loader.Custom.Sql.SqlQueryReturnProcessor for use by NHibernate.Loader.Custom.Sql.SqlQueryParser class. This functionality is now directly provided by the new NHibernate.Loader.Custom.Sql.SqlQueryContext class.

  Contains simplified code to transform INativeSQLQueryReturn instances into NHibernate.Loader.Custom.IReturn instances. This transformation code originates from the now defunct NHibernate.Loader.Custom.Sql.SqlQueryReturnProcessor class.

• NHibernate.Loader.Custom.CustomLoader

  The constructor code has been simplified a lot because of the reduced number of IReturn implementations.

[ggeurts]

JIRA issue created at https://nhibernate.jira.com/browse/NH-3303

[hazzik]

Hi, @ggeurts. Could you remove merge commit e6bbb04 from the pull request?

[ggeurts]

The merge commit has been removed.

[oskarb]

Hibernate seems to have done some serious Loader refactoring work in 2012-2014. Haven't looked at the details, but I worry that some of these changes might complicate porting. Any ideas on this?

[oskarb]

I've merged the first two commits (cc46650 and 2bd8bc7) and cherry-picked 71a4165 to master. The intervening 69cd430 causes test failures (at least on current master) , that I couldn't find any resolution for in the other commits (plus I wonder if those failures indicate this might be a breaking change for some users).

If more work on this is done, please rebase the remaining commits on current master. However, a comparison with the current state of the corresponding Hibernate classes should be made, so we don't divert from the structure too much without a very clear benefit.

[hazzik]

@oskarb can we avoid such long pull requests? I'll prefer to apply a patch from a pull request, or rebase on top of master.

[oskarb]

I assume that you mean that I merged commits that were two years old? Well I don't feel strongly about it. The benefit I thought was to avoid "duplicated" commits, i.e. git now knows that two commits from this PR was merged. Obviously it should not be done if the merge commit itself involves complicated conflict resolution. What drawbacks do you see?